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/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 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/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/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 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