From 7b5587094d432d7bbd73f11b054cf63d99e824fc Mon Sep 17 00:00:00 2001 From: anonpenguin23 Date: Mon, 15 Jun 2026 11:20:23 +0300 Subject: [PATCH] fix(gateway): api_key owners no longer 403 on namespaces they own MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The namespace-ownership middleware compared an api_key caller's RAW key against namespace_ownership.owner_id, but api_keys are stored HMAC-hashed (HashAPIKey). So every api_key-authenticated owner got a 403 on a namespace they actually own — blocking function deploy and PUT /v1/push/config. Hash the presented api_key before the ownership comparison (hashed first, raw second as a rolling-upgrade legacy fallback), mirroring the existing lookupAPIKeyNamespace pattern. The wallet path is unchanged (wallets stored raw). Security-reviewed: grants only to the correct key holder, no escalation. --- core/pkg/gateway/middleware.go | 43 ++++++++++++++++--- .../gateway/middleware_apikey_owner_test.go | 35 +++++++++++++++ 2 files changed, 73 insertions(+), 5 deletions(-) create mode 100644 core/pkg/gateway/middleware_apikey_owner_test.go diff --git a/core/pkg/gateway/middleware.go b/core/pkg/gateway/middleware.go index d02b9f2..c982502 100644 --- a/core/pkg/gateway/middleware.go +++ b/core/pkg/gateway/middleware.go @@ -844,14 +844,35 @@ func (g *Gateway) authorizationMiddleware(next http.Handler) http.Handler { nsID := nres.Rows[0][0] q := "SELECT 1 FROM namespace_ownership WHERE namespace_id = ? AND owner_type = ? AND owner_id = ? LIMIT 1" - res, err := db.Query(internalCtx, q, nsID, ownerType, ownerID) - // If primary owner check fails and we have a JWT wallet with API key fallback, try the API key - if (err != nil || res == nil || res.Count == 0) && ownerType == "wallet" && apiKeyFallback != "" { - res, err = db.Query(internalCtx, q, nsID, "api_key", apiKeyFallback) + // ownsBy reports whether (ot, oid) owns this namespace. API keys are + // stored HMAC-hashed in namespace_ownership (see service.HashAPIKey), + // while the presented value here is the RAW key — so for api_key owners + // we check the hashed form first and the raw form second (rolling- + // upgrade legacy), mirroring lookupAPIKeyNamespace. Without this, an + // api_key-authenticated owner never matched and got a 403 on a + // namespace they actually own (blocked function deploy / push config). + ownsBy := func(ot, oid string) bool { + hashed := "" + if ot == "api_key" { + hashed = g.authService.HashAPIKey(oid) + } + for _, c := range apiKeyOwnerCandidates(ot, oid, hashed) { + res, qerr := db.Query(internalCtx, q, nsID, ot, c) + if qerr == nil && res != nil && res.Count > 0 { + return true + } + } + return false } - if err != nil || res == nil || res.Count == 0 { + owns := ownsBy(ownerType, ownerID) + // A JWT wallet can also fall back to its API key (also stored hashed). + if !owns && ownerType == "wallet" && apiKeyFallback != "" { + owns = ownsBy("api_key", apiKeyFallback) + } + + if !owns { writeError(w, http.StatusForbidden, "forbidden: not an owner of namespace") return } @@ -860,6 +881,18 @@ func (g *Gateway) authorizationMiddleware(next http.Handler) http.Handler { }) } +// apiKeyOwnerCandidates returns the owner_id values to test for a namespace +// ownership check. API keys are stored HMAC-hashed in namespace_ownership, so +// for an api_key owner the hashed form is checked first and the raw key second +// (rolling-upgrade legacy, mirroring lookupAPIKeyNamespace). For every other +// owner type the value is used as-is. +func apiKeyOwnerCandidates(ownerType, ownerID, hashed string) []string { + if ownerType == "api_key" && hashed != "" && hashed != ownerID { + return []string{hashed, ownerID} + } + return []string{ownerID} +} + // requiresNamespaceOwnership returns true if the path should be guarded by // namespace ownership checks. func requiresNamespaceOwnership(p string) bool { diff --git a/core/pkg/gateway/middleware_apikey_owner_test.go b/core/pkg/gateway/middleware_apikey_owner_test.go new file mode 100644 index 0000000..5505f78 --- /dev/null +++ b/core/pkg/gateway/middleware_apikey_owner_test.go @@ -0,0 +1,35 @@ +package gateway + +import "testing" + +// Bugboard follow-up: api_key ownership is stored HMAC-hashed, but the +// ownership middleware presented the RAW key — so an api_key-authed owner got +// a 403 on a namespace they own (blocking function deploy / push config). +// apiKeyOwnerCandidates returns the values to check: hashed-first, raw-fallback. + +func TestApiKeyOwnerCandidates_apiKeyChecksHashedThenRaw(t *testing.T) { + got := apiKeyOwnerCandidates("api_key", "ak_raw", "HASHED") + if len(got) != 2 || got[0] != "HASHED" || got[1] != "ak_raw" { + t.Errorf("api_key: want [HASHED ak_raw] (hashed first, raw legacy fallback); got %v", got) + } +} + +func TestApiKeyOwnerCandidates_walletUsedAsIs(t *testing.T) { + got := apiKeyOwnerCandidates("wallet", "0xWALLET", "") + if len(got) != 1 || got[0] != "0xWALLET" { + t.Errorf("wallet must be used as-is (never hashed); got %v", got) + } +} + +func TestApiKeyOwnerCandidates_noHashAvailableFallsBackToRaw(t *testing.T) { + // When hashing is unavailable/disabled (HashAPIKey returns the key + // unchanged), don't duplicate — just check the raw value once. + got := apiKeyOwnerCandidates("api_key", "ak_raw", "ak_raw") + if len(got) != 1 || got[0] != "ak_raw" { + t.Errorf("no-op hash must yield a single raw candidate; got %v", got) + } + got2 := apiKeyOwnerCandidates("api_key", "ak_raw", "") + if len(got2) != 1 || got2[0] != "ak_raw" { + t.Errorf("empty hash must yield a single raw candidate; got %v", got2) + } +}