From e63c87c62b16e1020b135147886b28baee06363d Mon Sep 17 00:00:00 2001 From: admin Date: Wed, 28 Oct 2026 17:25:19 -0400 Subject: [PATCH 1/3] fix(workflows): manifest port matches the 3712 the service listens on (D-0821) plugin.json port and service.port said 3733 while the Dockerfile, compose fragment and config use 3712. d0821-test.sh pins the agreement and runs in the conformance job. --- free/workflows/plugin.json | 4 ++-- tests/conformance/d0821-test.sh | 18 ++++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) create mode 100755 tests/conformance/d0821-test.sh diff --git a/free/workflows/plugin.json b/free/workflows/plugin.json index c68fb3be9..77f46aca8 100644 --- a/free/workflows/plugin.json +++ b/free/workflows/plugin.json @@ -59,7 +59,7 @@ "*" ] }, - "port": 3733, + "port": 3712, "repository": "https://github.com/nself-org/plugins", "requires": { "nself": ">=1.0.0" @@ -151,7 +151,7 @@ "compose": "docker-compose.plugin.yml", "healthcheck": "/health", "kind": "compose", - "port": 3733 + "port": 3712 }, "status": "stable", "tables": [ diff --git a/tests/conformance/d0821-test.sh b/tests/conformance/d0821-test.sh new file mode 100755 index 000000000..785861f09 --- /dev/null +++ b/tests/conformance/d0821-test.sh @@ -0,0 +1,18 @@ +#!/usr/bin/env bash +# d0821-test.sh: regression proof for D-0821 on the REAL free/ tree. workflows listens on 3712 (Dockerfile EXPOSE, +# compose fragment, config default), so plugin.json port and service.port must say 3712 too, and no other plugin +# or reserved row may claim 3712. Needs jq only. +# Usage: d0821-test.sh +set -u +HERE=$(cd "$(dirname "$0")" && pwd); ROOT=$(cd "$HERE/../.." && pwd); rc=0 +no() { echo "FAIL d0821: $1"; rc=1; } +m=$ROOT/free/workflows/plugin.json +exp=$(awk '/^EXPOSE /{print $2}' "$ROOT/free/workflows/Dockerfile") +[ "$exp" = 3712 ] && echo "pass d0821: Dockerfile exposes 3712" || no "Dockerfile EXPOSE is '$exp', expected 3712" +grep -qF '127.0.0.1:3712:3712' "$ROOT/free/workflows/docker-compose.plugin.yml" && echo "pass d0821: compose fragment publishes 3712" || no "compose fragment does not publish 3712" +for k in .port .service.port; do + v=$(jq -r "$k" "$m") + [ "$v" = "$exp" ] && echo "pass d0821: plugin.json $k is $exp" || no "plugin.json $k is $v, the service listens on $exp" +done +out=$(bash "$ROOT/scripts/conformance/ports.sh" --plugins workflows 2>&1) && echo "pass d0821: no port collision for workflows" || { no "port collision"; printf '%s\n' "$out" | sed 's/^/ /'; } +[ $rc -eq 0 ] && echo "d0821: ok" || echo "d0821: FAIL"; exit $rc From f648f83cf8862db7378889e2c2d3a4210acc2185 Mon Sep 17 00:00:00 2001 From: admin Date: Wed, 28 Oct 2026 17:25:22 -0400 Subject: [PATCH 2/3] fix(ci): prune the BuildKit cache after each tarball build on CI (D-0973) build-from-tarball.sh now runs docker builder prune -f on exit when CI=true, so a shard of ~28 sequential Go image builds cannot fill the runner disk. A developer machine (CI unset) is never pruned. d0973-test.sh proves it with a stub docker. validate.yml runs both d0821 and d0973 checks. --- .github/workflows/validate.yml | 6 +++++ scripts/conformance/build-from-tarball.sh | 10 ++++++-- tests/conformance/d0973-test.sh | 28 +++++++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100755 tests/conformance/d0973-test.sh diff --git a/.github/workflows/validate.yml b/.github/workflows/validate.yml index 0af1a8032..56ad75f2a 100644 --- a/.github/workflows/validate.yml +++ b/.github/workflows/validate.yml @@ -393,6 +393,12 @@ jobs: - name: D-0815 forgejo and file-processing pass the whole gate run: bash tests/conformance/d0815-test.sh + - name: D-0973 the tarball build prunes the BuildKit cache on CI only + run: bash tests/conformance/d0973-test.sh + + - name: D-0821 workflows manifest port matches the port it listens on + run: bash tests/conformance/d0821-test.sh + - name: Released CLI v1.4.12 oracle run: bash scripts/conformance/v14-oracle.sh --fixtures diff --git a/scripts/conformance/build-from-tarball.sh b/scripts/conformance/build-from-tarball.sh index 7308bcd72..5a510cb46 100644 --- a/scripts/conformance/build-from-tarball.sh +++ b/scripts/conformance/build-from-tarball.sh @@ -1,7 +1,8 @@ #!/usr/bin/env bash # build-from-tarball.sh : deterministic-tar the plugin, extract it into an empty directory and # `docker build` there with the network disabled for RUN steps, so a build that needs anything not in the -# tarball (a replace target outside the dir, an unvendored module) fails here. Sequential; cleans its image. +# tarball (a replace target outside the dir, an unvendored module) fails here. Sequential; cleans its image and, +# when CI=true (a CI runner, never a developer machine), prunes the BuildKit cache so a long shard cannot fill the disk (D-0973). set -u . "$(dirname "$0")/lib.sh" dir=$(cd "$1" && pwd); name=$(plugin_name "$dir") @@ -13,7 +14,12 @@ if [ ! -f "$dir/Dockerfile" ]; then fi command -v docker >/dev/null 2>&1 && docker info >/dev/null 2>&1 || { fail build "$name" "docker is not available (a skipped build is a failure)"; exit 1; } work=$(conf_work)/tarball-$$; rm -rf "$work"; mkdir -p "$work/x"; tag=nself-conf-$name:$$ -trap 'rm -rf "$work"; docker rmi -f "$tag" >/dev/null 2>&1' EXIT +cleanup() { + rm -rf "$work"; docker rmi -f "$tag" >/dev/null 2>&1 + [ "${CI:-}" = true ] && docker builder prune -f >/dev/null 2>&1 + return 0 +} +trap cleanup EXIT bash "$CONF_ROOT/scripts/deterministic-tar.sh" "$work/p.tar.gz" -C "$(dirname "$dir")" "$(basename "$dir")" >/dev/null 2>&1 || { fail build "$name" "deterministic-tar failed"; exit 1; } tar -xzf "$work/p.tar.gz" -C "$work/x" # P7-PLUG-44 ruling: tests/boot/build-network (empty marker, same file the boot kit honours) lifts the network ban for a diff --git a/tests/conformance/d0973-test.sh b/tests/conformance/d0973-test.sh new file mode 100755 index 000000000..ab3e83969 --- /dev/null +++ b/tests/conformance/d0973-test.sh @@ -0,0 +1,28 @@ +#!/usr/bin/env bash +# d0973-test.sh: regression proof for D-0973. build-from-tarball.sh prunes the BuildKit cache on exit only when +# CI=true (a runner), never on a developer machine, and also on a failed build. A stub `docker` on PATH records +# its calls; no real Docker or network is used. Usage: d0973-test.sh +set -u +HERE=$(cd "$(dirname "$0")" && pwd); ROOT=$(cd "$HERE/../.." && pwd); rc=0 +no() { echo "FAIL d0973: $1"; rc=1; } +T=$(mktemp -d "${TMPDIR:-/tmp}/d0973.XXXXXX"); trap 'rm -rf "$T"' EXIT +mkdir -p "$T/bin" "$T/plugins/demo" +printf '{"name":"demo"}\n' > "$T/plugins/demo/plugin.json"; printf 'FROM scratch\n' > "$T/plugins/demo/Dockerfile" +cat > "$T/bin/docker" <<'STUB' +#!/bin/sh +echo "$*" >> "$STUB_LOG" +[ "$1" = build ] && [ -n "${STUB_BUILD_FAIL:-}" ] && { echo "boom"; exit 1; } +exit 0 +STUB +chmod +x "$T/bin/docker" +run() { # CI value (empty = unset), build-fail flag + : > "$T/log"; env -u CI ${1:+CI=$1} STUB_LOG="$T/log" ${2:+STUB_BUILD_FAIL=1} CONF_WORKDIR="$T/work" PATH="$T/bin:$PATH" \ + bash "$ROOT/scripts/conformance/build-from-tarball.sh" "$T/plugins/demo" >/dev/null 2>&1 +} +pruned() { grep -q '^builder prune -f' "$T/log"; } +run true; pruned && echo "pass d0973: CI=true prunes the builder cache" || no "CI=true did not run docker builder prune -f" +run true 1; pruned && echo "pass d0973: CI=true prunes after a failed build" || no "CI=true did not prune after a failed build" +run ""; pruned && no "CI unset ran docker builder prune (developer machine)" || echo "pass d0973: CI unset never prunes" +run false; pruned && no "CI=false ran docker builder prune" || echo "pass d0973: CI=false never prunes" +run true; grep -q '^rmi -f' "$T/log" && echo "pass d0973: the image is still removed" || no "docker rmi -f missing" +[ $rc -eq 0 ] && echo "d0973: ok" || echo "d0973: FAIL"; exit $rc From bfa6201aa5db20f94586ca325f436aed8fc314f2 Mon Sep 17 00:00:00 2001 From: admin Date: Wed, 28 Oct 2026 17:25:26 -0400 Subject: [PATCH 3/3] fix(plugins): one inter-plugin auth header, X-Internal-Token, with a compatibility read of the old names (D-0198) audit-log, storage, post and linkedin accept X-Internal-Token and still read X-Plugin-Secret, X-Internal-Secret and X-Internal-Key. audit-log and storage compare in constant time and emit the canonical name in errors. The storage Go client sends the canonical header plus the legacy one for one release. Callers in bundles and plugins-pro are outside this repo. --- free/audit-log/internal/handlers.go | 6 +-- free/audit-log/internal/handlers_admin.go | 9 ++-- free/audit-log/internal/handlers_helpers.go | 2 +- free/audit-log/internal/handlers_ingest.go | 9 ++-- free/audit-log/internal/handlers_list_get.go | 14 +++--- free/audit-log/internal/handlers_test.go | 2 +- free/audit-log/internal/internal_auth.go | 37 +++++++++++++++ free/audit-log/internal/internal_auth_test.go | 46 +++++++++++++++++++ free/audit-log/nginx/audit-log.conf | 2 +- free/linkedin/internal/handlers.go | 4 +- free/linkedin/internal/internal_auth.go | 20 ++++++++ free/linkedin/internal/internal_auth_test.go | 37 +++++++++++++++ free/post/cmd/internal_auth_test.go | 37 +++++++++++++++ free/post/cmd/main.go | 21 ++++++++- free/storage/README.md | 15 +++--- free/storage/client/client.go | 12 +++-- free/storage/internal/auth.go | 23 ++++++++-- free/storage/internal/auth_test.go | 29 ++++++++++++ free/storage/internal/client_e2e_test.go | 4 +- free/storage/internal/config.go | 2 +- free/storage/internal/handlers.go | 2 +- 21 files changed, 287 insertions(+), 46 deletions(-) create mode 100644 free/audit-log/internal/internal_auth.go create mode 100644 free/audit-log/internal/internal_auth_test.go create mode 100644 free/linkedin/internal/internal_auth.go create mode 100644 free/linkedin/internal/internal_auth_test.go create mode 100644 free/post/cmd/internal_auth_test.go diff --git a/free/audit-log/internal/handlers.go b/free/audit-log/internal/handlers.go index e0cfece2d..9c0ccf265 100644 --- a/free/audit-log/internal/handlers.go +++ b/free/audit-log/internal/handlers.go @@ -14,8 +14,8 @@ import ( // RegisterRoutes mounts all audit-log endpoints on the given router. // // - secret is the value of PLUGIN_INTERNAL_SECRET; all /events and -// /admin/events endpoints require a matching X-Plugin-Secret header, except -// /admin/events which accepts either X-Plugin-Secret or X-Hasura-Admin-Secret. +// /admin/events endpoints require a matching X-Internal-Token header, except +// /admin/events which accepts either X-Internal-Token or X-Hasura-Admin-Secret. // - adminSecret is the value of HASURA_GRAPHQL_ADMIN_SECRET; when non-empty // GET /admin/events also accepts requests carrying this header. func RegisterRoutes(r chi.Router, pool *pgxpool.Pool, secret, adminSecret string) { @@ -70,4 +70,4 @@ func handleHealth(pool *pgxpool.Pool) http.HandlerFunc { // handleIngest handles POST /events. // This endpoint is for internal use only (e.g. other plugins, the CLI, Admin). -// All requests must include a matching X-Plugin-Secret header. +// All requests must include a matching X-Internal-Token header. diff --git a/free/audit-log/internal/handlers_admin.go b/free/audit-log/internal/handlers_admin.go index 615a7ffb8..8efc3873d 100644 --- a/free/audit-log/internal/handlers_admin.go +++ b/free/audit-log/internal/handlers_admin.go @@ -14,7 +14,7 @@ func handleAdminList(pool *pgxpool.Pool, secret, adminSecret string) http.Handle return func(w http.ResponseWriter, r *http.Request) { if !authorizeAdminRequest(r, secret, adminSecret) { sdk.Respond(w, http.StatusUnauthorized, map[string]string{ - "error": "missing or invalid authentication header; provide X-Plugin-Secret or X-Hasura-Admin-Secret", + "error": "missing or invalid authentication header; provide X-Internal-Token or X-Hasura-Admin-Secret", }) return } @@ -64,9 +64,8 @@ func handleAdminList(pool *pgxpool.Pool, secret, adminSecret string) http.Handle // Size-cap exception: single-responsibility HTTP route handler — 108L of request decode + validate + DB op + response encode; splitting adds indirection without cohesion gain. func handleExport(pool *pgxpool.Pool, secret string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - provided := r.Header.Get("X-Plugin-Secret") - if provided == "" || provided != secret { - sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Plugin-Secret"}) + if !hasInternalSecret(r, secret) { + sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Internal-Token"}) return } @@ -172,5 +171,5 @@ func handleExport(pool *pgxpool.Pool, secret string) http.HandlerFunc { } // authorizeAdminRequest returns true when the request carries a valid -// X-Plugin-Secret or (when adminSecret is non-empty) a valid +// X-Internal-Token or (when adminSecret is non-empty) a valid // X-Hasura-Admin-Secret header. diff --git a/free/audit-log/internal/handlers_helpers.go b/free/audit-log/internal/handlers_helpers.go index e885ebd11..46256d5d1 100644 --- a/free/audit-log/internal/handlers_helpers.go +++ b/free/audit-log/internal/handlers_helpers.go @@ -9,7 +9,7 @@ import ( ) func authorizeAdminRequest(r *http.Request, secret, adminSecret string) bool { - if ps := r.Header.Get("X-Plugin-Secret"); ps != "" && ps == secret { + if hasInternalSecret(r, secret) { return true } if adminSecret != "" { diff --git a/free/audit-log/internal/handlers_ingest.go b/free/audit-log/internal/handlers_ingest.go index 2cab02186..1c4bcfbfb 100644 --- a/free/audit-log/internal/handlers_ingest.go +++ b/free/audit-log/internal/handlers_ingest.go @@ -15,9 +15,8 @@ import ( func handleIngest(pool *pgxpool.Pool, secret string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { // Always authenticate internal callers. - provided := r.Header.Get("X-Plugin-Secret") - if provided == "" || provided != secret { - sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Plugin-Secret"}) + if !hasInternalSecret(r, secret) { + sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Internal-Token"}) return } @@ -89,7 +88,7 @@ func handleIngest(pool *pgxpool.Pool, secret string) http.HandlerFunc { CreatedAt: time.Now().UTC(), } - // Prefer real client IP when X-Plugin-Secret is set (trusted call from + // Prefer real client IP when the internal secret header is set (trusted call from // within the stack) or fall back to the remote address. if event.IPAddress == "" { event.IPAddress = realIP(r) @@ -111,7 +110,7 @@ func handleIngest(pool *pgxpool.Pool, secret string) http.HandlerFunc { } // handleList handles GET /events. -// All requests must include a matching X-Plugin-Secret header. +// All requests must include a matching X-Internal-Token header. // // Supported query parameters: // diff --git a/free/audit-log/internal/handlers_list_get.go b/free/audit-log/internal/handlers_list_get.go index 1fdcfcbbe..1bb73e056 100644 --- a/free/audit-log/internal/handlers_list_get.go +++ b/free/audit-log/internal/handlers_list_get.go @@ -11,9 +11,8 @@ import ( func handleList(pool *pgxpool.Pool, secret string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - provided := r.Header.Get("X-Plugin-Secret") - if provided == "" || provided != secret { - sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Plugin-Secret"}) + if !hasInternalSecret(r, secret) { + sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Internal-Token"}) return } @@ -46,12 +45,11 @@ func handleList(pool *pgxpool.Pool, secret string) http.HandlerFunc { } // handleGet handles GET /events/{id}. -// All requests must include a matching X-Plugin-Secret header. +// All requests must include a matching X-Internal-Token header. func handleGet(pool *pgxpool.Pool, secret string) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - provided := r.Header.Get("X-Plugin-Secret") - if provided == "" || provided != secret { - sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Plugin-Secret"}) + if !hasInternalSecret(r, secret) { + sdk.Respond(w, http.StatusUnauthorized, map[string]string{"error": "missing or invalid X-Internal-Token"}) return } @@ -76,5 +74,5 @@ func handleGet(pool *pgxpool.Pool, secret string) http.HandlerFunc { // handleAdminList handles GET /admin/events. // This is the Admin-facing variant of GET /events. It accepts either -// X-Plugin-Secret (plugin internal callers) or X-Hasura-Admin-Secret (Admin +// X-Internal-Token (plugin internal callers) or X-Hasura-Admin-Secret (Admin // UI via Hasura). Supports the same filter query parameters as GET /events. diff --git a/free/audit-log/internal/handlers_test.go b/free/audit-log/internal/handlers_test.go index 5140a2e86..dad57a1e7 100644 --- a/free/audit-log/internal/handlers_test.go +++ b/free/audit-log/internal/handlers_test.go @@ -8,7 +8,7 @@ import ( ) // TestAuthorizeAdminRequest_PluginSecret verifies that a valid X-Plugin-Secret -// header grants access. +// header (legacy name, still read) grants access. func TestAuthorizeAdminRequest_PluginSecret(t *testing.T) { r := httptest.NewRequest(http.MethodGet, "/", nil) r.Header.Set("X-Plugin-Secret", "my-secret") diff --git a/free/audit-log/internal/internal_auth.go b/free/audit-log/internal/internal_auth.go new file mode 100644 index 000000000..d4e146c59 --- /dev/null +++ b/free/audit-log/internal/internal_auth.go @@ -0,0 +1,37 @@ +package internal + +import ( + "crypto/sha256" + "crypto/subtle" + "net/http" +) + +// InternalSecretHeader is the one header that carries PLUGIN_INTERNAL_SECRET on inter-plugin calls (D-0198). +const InternalSecretHeader = "X-Internal-Token" + +// legacySecretHeaders are the older names for the same check, still read so callers that have not moved to +// InternalSecretHeader keep working. Remove them once every caller sends the canonical name. +var legacySecretHeaders = []string{"X-Plugin-Secret", "X-Internal-Secret", "X-Internal-Key"} + +// presentedSecret returns the secret a request carries: the canonical header, else the first legacy header set. +func presentedSecret(r *http.Request) string { + if v := r.Header.Get(InternalSecretHeader); v != "" { + return v + } + for _, h := range legacySecretHeaders { + if v := r.Header.Get(h); v != "" { + return v + } + } + return "" +} + +// hasInternalSecret reports whether the request carries a non-empty secret equal to want, compared in constant time. +func hasInternalSecret(r *http.Request, want string) bool { + got := presentedSecret(r) + if got == "" || want == "" { + return false + } + g, w := sha256.Sum256([]byte(got)), sha256.Sum256([]byte(want)) + return subtle.ConstantTimeCompare(g[:], w[:]) == 1 +} diff --git a/free/audit-log/internal/internal_auth_test.go b/free/audit-log/internal/internal_auth_test.go new file mode 100644 index 000000000..a3f9a1b00 --- /dev/null +++ b/free/audit-log/internal/internal_auth_test.go @@ -0,0 +1,46 @@ +package internal + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// TestHasInternalSecret proves the canonical header is accepted, every legacy name is still read (D-0198), and a +// missing, empty or wrong secret is refused, including when the configured secret is empty. +func TestHasInternalSecret(t *testing.T) { + cases := []struct { + name string + headers map[string]string + want string + ok bool + }{ + {"canonical", map[string]string{"X-Internal-Token": "s3"}, "s3", true}, + {"legacy X-Plugin-Secret", map[string]string{"X-Plugin-Secret": "s3"}, "s3", true}, + {"legacy X-Internal-Secret", map[string]string{"X-Internal-Secret": "s3"}, "s3", true}, + {"legacy X-Internal-Key", map[string]string{"X-Internal-Key": "s3"}, "s3", true}, + {"canonical wins over a legacy header", map[string]string{"X-Internal-Token": "s3", "X-Plugin-Secret": "other"}, "s3", true}, + {"wrong canonical is not rescued by a legacy header", map[string]string{"X-Internal-Token": "bad", "X-Plugin-Secret": "s3"}, "s3", false}, + {"wrong value", map[string]string{"X-Internal-Token": "bad"}, "s3", false}, + {"no header", nil, "s3", false}, + {"empty configured secret never matches", map[string]string{"X-Internal-Token": ""}, "", false}, + } + for _, c := range cases { + r := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range c.headers { + r.Header.Set(k, v) + } + if got := hasInternalSecret(r, c.want); got != c.ok { + t.Errorf("%s: hasInternalSecret = %v, want %v", c.name, got, c.ok) + } + } +} + +// TestAuthorizeAdminRequest_CanonicalHeader proves the admin gate accepts X-Internal-Token. +func TestAuthorizeAdminRequest_CanonicalHeader(t *testing.T) { + r := httptest.NewRequest(http.MethodGet, "/", nil) + r.Header.Set("X-Internal-Token", "my-secret") + if !authorizeAdminRequest(r, "my-secret", "") { + t.Error("expected authorizeAdminRequest to accept X-Internal-Token") + } +} diff --git a/free/audit-log/nginx/audit-log.conf b/free/audit-log/nginx/audit-log.conf index aea6320cf..af296f03b 100644 --- a/free/audit-log/nginx/audit-log.conf +++ b/free/audit-log/nginx/audit-log.conf @@ -10,7 +10,7 @@ server { set $upstream http://audit-log:3308; # POST /events is an internal ingestion endpoint. - # It is protected by X-Plugin-Secret at the application layer. + # It is protected by X-Internal-Token at the application layer. # At the nginx layer we additionally restrict it to requests originating # from the internal Docker network (127.0.0.0/8 and 172.16.0.0/12). location = /events { diff --git a/free/linkedin/internal/handlers.go b/free/linkedin/internal/handlers.go index e8f107c19..b25b19b18 100644 --- a/free/linkedin/internal/handlers.go +++ b/free/linkedin/internal/handlers.go @@ -312,11 +312,11 @@ func (h *Handlers) HandleTools(w http.ResponseWriter, _ *http.Request) { // ============================================================================= // InternalAuthMiddleware rejects requests that do not carry the correct -// X-Internal-Token header. +// X-Internal-Token header (or a legacy name, see presentedSecret). func InternalAuthMiddleware(secret string) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := r.Header.Get("X-Internal-Token") + token := presentedSecret(r) if token == "" { http.Error(w, "X-Internal-Token required", http.StatusUnauthorized) return diff --git a/free/linkedin/internal/internal_auth.go b/free/linkedin/internal/internal_auth.go new file mode 100644 index 000000000..688938b16 --- /dev/null +++ b/free/linkedin/internal/internal_auth.go @@ -0,0 +1,20 @@ +package internal + +import "net/http" + +// legacySecretHeaders are the older names for the X-Internal-Token check (D-0198), still read so callers that have +// not moved to the canonical name keep working. Remove them once every caller sends X-Internal-Token. +var legacySecretHeaders = []string{"X-Plugin-Secret", "X-Internal-Secret", "X-Internal-Key"} + +// presentedSecret returns the secret a request carries: X-Internal-Token, else the first legacy header set. +func presentedSecret(r *http.Request) string { + if v := r.Header.Get("X-Internal-Token"); v != "" { + return v + } + for _, h := range legacySecretHeaders { + if v := r.Header.Get(h); v != "" { + return v + } + } + return "" +} diff --git a/free/linkedin/internal/internal_auth_test.go b/free/linkedin/internal/internal_auth_test.go new file mode 100644 index 000000000..2ef3b9ec6 --- /dev/null +++ b/free/linkedin/internal/internal_auth_test.go @@ -0,0 +1,37 @@ +package internal + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// TestInternalAuth_HeaderNames proves X-Internal-Token and every legacy name carry the same secret (D-0198), a +// wrong value under any name is refused, and a legacy header never overrides a present canonical one. +func TestInternalAuth_HeaderNames(t *testing.T) { + cases := []struct { + name string + headers map[string]string + want int + }{ + {"canonical", map[string]string{"X-Internal-Token": "s3"}, 200}, + {"legacy X-Plugin-Secret", map[string]string{"X-Plugin-Secret": "s3"}, 200}, + {"legacy X-Internal-Secret", map[string]string{"X-Internal-Secret": "s3"}, 200}, + {"legacy X-Internal-Key", map[string]string{"X-Internal-Key": "s3"}, 200}, + {"wrong canonical is not rescued by a right legacy header", map[string]string{"X-Internal-Token": "bad", "X-Plugin-Secret": "s3"}, 403}, + {"wrong legacy", map[string]string{"X-Plugin-Secret": "bad"}, 403}, + {"no header", nil, 401}, + } + for _, c := range cases { + h := InternalAuthMiddleware("s3")(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + req := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range c.headers { + req.Header.Set(k, v) + } + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + if rr.Code != c.want { + t.Errorf("%s: code %d, want %d", c.name, rr.Code, c.want) + } + } +} diff --git a/free/post/cmd/internal_auth_test.go b/free/post/cmd/internal_auth_test.go new file mode 100644 index 000000000..5ddbb12f2 --- /dev/null +++ b/free/post/cmd/internal_auth_test.go @@ -0,0 +1,37 @@ +package main + +import ( + "net/http" + "net/http/httptest" + "testing" +) + +// TestInternalAuth_HeaderNames proves X-Internal-Token and every legacy name carry the same secret (D-0198), a +// wrong value under any name is refused, and a legacy header never overrides a present canonical one. +func TestInternalAuth_HeaderNames(t *testing.T) { + cases := []struct { + name string + headers map[string]string + want int + }{ + {"canonical", map[string]string{"X-Internal-Token": "s3"}, 200}, + {"legacy X-Plugin-Secret", map[string]string{"X-Plugin-Secret": "s3"}, 200}, + {"legacy X-Internal-Secret", map[string]string{"X-Internal-Secret": "s3"}, 200}, + {"legacy X-Internal-Key", map[string]string{"X-Internal-Key": "s3"}, 200}, + {"wrong canonical is not rescued by a right legacy header", map[string]string{"X-Internal-Token": "bad", "X-Plugin-Secret": "s3"}, 403}, + {"wrong legacy", map[string]string{"X-Plugin-Secret": "bad"}, 403}, + {"no header", nil, 401}, + } + for _, c := range cases { + h := internalAuthMiddleware("s3")(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + req := httptest.NewRequest(http.MethodGet, "/", nil) + for k, v := range c.headers { + req.Header.Set(k, v) + } + rr := httptest.NewRecorder() + h.ServeHTTP(rr, req) + if rr.Code != c.want { + t.Errorf("%s: code %d, want %d", c.name, rr.Code, c.want) + } + } +} diff --git a/free/post/cmd/main.go b/free/post/cmd/main.go index a7392229f..7a312000f 100644 --- a/free/post/cmd/main.go +++ b/free/post/cmd/main.go @@ -104,12 +104,29 @@ func main() { log.Println("nself-post stopped") } +// legacySecretHeaders are the older names for the X-Internal-Token check (D-0198), still read so callers that have +// not moved to the canonical name keep working. Remove them once every caller sends X-Internal-Token. +var legacySecretHeaders = []string{"X-Plugin-Secret", "X-Internal-Secret", "X-Internal-Key"} + +// presentedSecret returns the secret a request carries: X-Internal-Token, else the first legacy header set. +func presentedSecret(r *http.Request) string { + if v := r.Header.Get("X-Internal-Token"); v != "" { + return v + } + for _, h := range legacySecretHeaders { + if v := r.Header.Get(h); v != "" { + return v + } + } + return "" +} + // internalAuthMiddleware rejects requests that do not present a valid -// X-Internal-Token header matching the configured secret. +// X-Internal-Token header (or a legacy name, see presentedSecret) matching the configured secret. func internalAuthMiddleware(secret string) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - token := r.Header.Get("X-Internal-Token") + token := presentedSecret(r) if token == "" { http.Error(w, "X-Internal-Token required", http.StatusUnauthorized) return diff --git a/free/storage/README.md b/free/storage/README.md index 7b42580f1..f3b5c9579 100644 --- a/free/storage/README.md +++ b/free/storage/README.md @@ -38,7 +38,7 @@ Check this plugin with `curl http://127.0.0.1:9007/health`. |----------|----------|---------|-------------| | `NSELF_DB_URL` | Yes | — | PostgreSQL connection string | | `STORAGE_PLUGIN_PORT` | No | `9007` | HTTP server port | -| `PLUGIN_INTERNAL_SECRET` | For `/storage/*` | — | Shared secret every `/storage/*` request must carry in `X-Plugin-Secret`. The value comes from the project's `PLUGIN_INTERNAL_SECRET` (written by `nself init`); when it is empty or blank, every `/storage/*` call answers `503` | +| `PLUGIN_INTERNAL_SECRET` | For `/storage/*` | — | Shared secret every `/storage/*` request must carry in `X-Internal-Token`. The value comes from the project's `PLUGIN_INTERNAL_SECRET` (written by `nself init`); when it is empty or blank, every `/storage/*` call answers `503` | | `STORAGE_MAX_OBJECT_BYTES` | No | `5368709120` (5 GiB) | Largest accepted object, positive integer bytes; larger uploads answer `413` | | `STORAGE_ACCOUNT_QUOTA_BYTES` | No | `0` (off) | Total bytes one account may store, non-negative integer bytes; `0` or unset disables the quota | | `STORAGE_MAX_UPLOADS` | No | `64` | Uploads of any size that may run at once; others wait | @@ -59,12 +59,13 @@ reaches S3 and fails there answers `502 {"error":"object storage error"}`. ## Authentication The `/storage/*` routes are for trusted backend services, not end users. Every request must carry -`X-Plugin-Secret: ` (compared in constant time). The check runs before anything else, so the +`X-Internal-Token: ` (compared in constant time). The older names `X-Plugin-Secret`, +`X-Internal-Secret` and `X-Internal-Key` are still read for callers that have not moved, but new callers use `X-Internal-Token`. The check runs before anything else, so the `X-Hasura-Source-Account-Id` header, which picks the tenant, is trusted only on requests that carry the secret. | Case | Answer | |------|--------| -| `X-Plugin-Secret` missing or wrong | `401 {"error":"unauthorized"}` | +| `X-Internal-Token` missing or wrong | `401 {"error":"unauthorized"}` | | `PLUGIN_INTERNAL_SECRET` not set on the plugin | `503 {"error":"PLUGIN_INTERNAL_SECRET not configured"}` (fail closed; the plugin logs one line at start and `/health` stays up) | The value is the project's `PLUGIN_INTERNAL_SECRET` (written to `.env` by `nself init`). If a project has no value, the @@ -80,7 +81,7 @@ Do not hand the secret to browsers or mobile apps: a caller holding it can name Every server-side caller sends the header itself. Read the value from `PLUGIN_INTERNAL_SECRET` in the caller's environment; never put it in a repo, a browser bundle or a mobile app. -- **Go:** `github.com/nself-org/plugin-storage/client` sets `X-Plugin-Secret` (and optionally +- **Go:** `github.com/nself-org/plugin-storage/client` sets `X-Internal-Token` (plus the legacy `X-Plugin-Secret` for one release, and optionally `X-Hasura-Source-Account-Id`) on every call: ```go @@ -93,13 +94,13 @@ environment; never put it in a repo, a browser bundle or a mobile app. ```yaml headers: - - name: X-Plugin-Secret + - name: X-Internal-Token value_from_env: PLUGIN_INTERNAL_SECRET ``` The Hasura container needs `PLUGIN_INTERNAL_SECRET` in its environment. -- **Any other HTTP client:** send `X-Plugin-Secret: $PLUGIN_INTERNAL_SECRET` on every `/storage/*` request. +- **Any other HTTP client:** send `X-Internal-Token: $PLUGIN_INTERNAL_SECRET` on every `/storage/*` request. ## API @@ -232,7 +233,7 @@ To point the suite at another endpoint, set `S3COV_ENDPOINT`, `S3COV_ACCESS_KEY` - Object keys are validated against path traversal (`..` sequences are rejected). - `S3_ENDPOINT` is configuration-only; no request parameter can override the endpoint. - S3 credentials come from the environment only. -- `/storage/*` requires `X-Plugin-Secret` (see Authentication); the account header is trusted only with it. +- `/storage/*` requires `X-Internal-Token` (see Authentication); the account header is trusted only with it. - All DB queries are scoped to `source_account_id` for multi-app isolation, and S3 keys carry the same prefix. - Hasura row filters enforce `source_account_id = X-Hasura-Source-Account-Id` on all tables. diff --git a/free/storage/client/client.go b/free/storage/client/client.go index 77b3265f1..868ac25fe 100644 --- a/free/storage/client/client.go +++ b/free/storage/client/client.go @@ -1,6 +1,6 @@ // Package client is the Go caller for the storage plugin's /storage/* HTTP API. // -// Purpose: send X-Plugin-Secret (PLUGIN_INTERNAL_SECRET) on every call, so a server-side caller is not refused +// Purpose: send X-Internal-Token (PLUGIN_INTERNAL_SECRET) on every call, so a server-side caller is not refused // with 401 by the plugin's internal-secret gate (D-0396, D-0648). // Inputs: the plugin base URL, the internal secret, an optional source account id. // Outputs: typed results, or *StatusError for any non-2xx answer. @@ -22,9 +22,12 @@ import ( ) const ( - secretHeader = "X-Plugin-Secret" - accountHeader = "X-Hasura-Source-Account-Id" - maxErrBody = 4 << 10 + secretHeader = "X-Internal-Token" + // legacySecretHeader is sent as well for one release, so this client still works against a storage plugin that + // only reads the old name (D-0198). + legacySecretHeader = "X-Plugin-Secret" + accountHeader = "X-Hasura-Source-Account-Id" + maxErrBody = 4 << 10 ) // ErrNoSecret is returned when no internal secret is available. Failing here is clearer than a 401 or 503 later. @@ -84,6 +87,7 @@ func (c *Client) Do(ctx context.Context, method, path, contentType string, body req.ContentLength = length } req.Header.Set(secretHeader, c.secret) + req.Header.Set(legacySecretHeader, c.secret) if c.account != "" { req.Header.Set(accountHeader, c.account) } diff --git a/free/storage/internal/auth.go b/free/storage/internal/auth.go index 229c804c5..40624f085 100644 --- a/free/storage/internal/auth.go +++ b/free/storage/internal/auth.go @@ -6,8 +6,25 @@ import ( "net/http" ) -// secretHeader carries PLUGIN_INTERNAL_SECRET on every /storage/* request. -const secretHeader = "X-Plugin-Secret" +// secretHeader carries PLUGIN_INTERNAL_SECRET on every /storage/* request (D-0198: one name across plugins). +const secretHeader = "X-Internal-Token" + +// legacySecretHeaders are the older names for the same check, still read so callers that have not moved to +// secretHeader keep working. Remove them once every caller sends the canonical name. +var legacySecretHeaders = []string{"X-Plugin-Secret", "X-Internal-Secret", "X-Internal-Key"} + +// presentedSecret returns the secret a request carries: the canonical header, else the first legacy header set. +func presentedSecret(r *http.Request) string { + if v := r.Header.Get(secretHeader); v != "" { + return v + } + for _, h := range legacySecretHeaders { + if v := r.Header.Get(h); v != "" { + return v + } + } + return "" +} // secretEqual compares two secrets in constant time. Both sides are hashed first so the comparison takes the same // time for any lengths (ConstantTimeCompare alone returns early on a length mismatch). @@ -34,7 +51,7 @@ func (s *Server) requireInternalSecret(next http.Handler) http.Handler { http.Error(w, `{"error":"PLUGIN_INTERNAL_SECRET not configured"}`, http.StatusServiceUnavailable) return } - if !secretEqual(r.Header.Get(secretHeader), want) { + if !secretEqual(presentedSecret(r), want) { http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized) return } diff --git a/free/storage/internal/auth_test.go b/free/storage/internal/auth_test.go index 5da491d9d..bf98bd147 100644 --- a/free/storage/internal/auth_test.go +++ b/free/storage/internal/auth_test.go @@ -86,3 +86,32 @@ func TestStorageRoutes_forgedAccountHeaderNeverReachesHandler(t *testing.T) { } func ptr(s string) *string { return &s } + +// D-0198: the canonical X-Internal-Token and every legacy name carry the same secret; a legacy header never +// overrides a present canonical one, and a wrong value under any name is refused. +func TestRequireInternalSecret_headerNames(t *testing.T) { + cases := []struct { + name string + headers map[string]string + want int + }{ + {"canonical", map[string]string{"X-Internal-Token": testSecret}, 200}, + {"legacy X-Plugin-Secret", map[string]string{"X-Plugin-Secret": testSecret}, 200}, + {"legacy X-Internal-Secret", map[string]string{"X-Internal-Secret": testSecret}, 200}, + {"legacy X-Internal-Key", map[string]string{"X-Internal-Key": testSecret}, 200}, + {"wrong canonical is not rescued by a right legacy header", map[string]string{"X-Internal-Token": "test-wrong-secret", "X-Plugin-Secret": testSecret}, 401}, + {"wrong legacy", map[string]string{"X-Plugin-Secret": "test-wrong-secret"}, 401}, + } + for _, c := range cases { + s := &Server{cfg: &Config{InternalSecret: testSecret}} + req := httptest.NewRequest(http.MethodGet, "/storage/buckets", nil) + for k, v := range c.headers { + req.Header.Set(k, v) + } + rr := httptest.NewRecorder() + s.requireInternalSecret(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})).ServeHTTP(rr, req) + if rr.Code != c.want { + t.Errorf("%s: code %d, want %d", c.name, rr.Code, c.want) + } + } +} diff --git a/free/storage/internal/client_e2e_test.go b/free/storage/internal/client_e2e_test.go index 2ad807024..ba00b3874 100644 --- a/free/storage/internal/client_e2e_test.go +++ b/free/storage/internal/client_e2e_test.go @@ -22,7 +22,7 @@ func serveRoutes(t *testing.T, secret string) *httptest.Server { } // TestClient_sendsSecretThroughGeneratedRoute is the D-0648 end-to-end check: the shipped client sends -// X-Plugin-Secret on every call, so storage answers it instead of refusing with 401. +// X-Internal-Token (and the legacy X-Plugin-Secret) on every call, so storage answers it instead of refusing with 401. func TestClient_sendsSecretThroughGeneratedRoute(t *testing.T) { srv := serveRoutes(t, "route-secret") c, err := client.New(srv.URL, "route-secret") @@ -68,7 +68,7 @@ func TestClient_sendsAccountHeader(t *testing.T) { if err := c.DeleteObject(context.Background(), "b", "k"); err != nil { t.Fatal(err) } - if got.Get("X-Plugin-Secret") != "s3cret" || got.Get("X-Hasura-Source-Account-Id") != "tenant-7" { + if got.Get("X-Internal-Token") != "s3cret" || got.Get("X-Plugin-Secret") != "s3cret" || got.Get("X-Hasura-Source-Account-Id") != "tenant-7" { t.Fatalf("headers = %v", got) } } diff --git a/free/storage/internal/config.go b/free/storage/internal/config.go index f3c5d4fe0..3cbd87cfa 100644 --- a/free/storage/internal/config.go +++ b/free/storage/internal/config.go @@ -33,7 +33,7 @@ type Config struct { MaxObjectBytes int64 // AccountQuotaBytes caps the summed size of one account's objects (STORAGE_ACCOUNT_QUOTA_BYTES); 0 = off. AccountQuotaBytes int64 - // InternalSecret is PLUGIN_INTERNAL_SECRET. /storage/* routes require it in X-Plugin-Secret; empty = routes + // InternalSecret is PLUGIN_INTERNAL_SECRET. /storage/* routes require it in X-Internal-Token; empty = routes // answer 503 (fail closed). Surrounding whitespace is trimmed, so a whitespace-only value counts as unset (a header // value never carries it, so such a secret could never match and every call would answer 401 with no hint). InternalSecret string diff --git a/free/storage/internal/handlers.go b/free/storage/internal/handlers.go index 42fa6723f..baeb62705 100644 --- a/free/storage/internal/handlers.go +++ b/free/storage/internal/handlers.go @@ -9,7 +9,7 @@ // - Object bytes never touch Postgres; with no S3 credentials configured object routes answer 503. // - Object keys are validated to block path traversal. // - source_account_id from X-Hasura-Source-Account-Id header scopes all DB queries. The header is trusted only on -// requests that carry X-Plugin-Secret equal to PLUGIN_INTERNAL_SECRET (callers are trusted backend services): +// requests that carry X-Internal-Token (or a legacy name, see auth.go) equal to PLUGIN_INTERNAL_SECRET (callers are trusted backend services): // wrong or missing secret is 401, secret not configured is 503 (see auth.go). package internal