From c42808ffa6a5e3fe1553c389abb10bb9f7b1041e Mon Sep 17 00:00:00 2001 From: 42-v <210031137+42-v@users.noreply.github.com> Date: Wed, 26 Aug 2026 21:32:01 +0200 Subject: [PATCH 1/2] fix(erasure): an erased subject cannot write where SQL cannot see it PUT /user/profile was closed by making auth.users refuse the write itself. The other subject-writing routes cannot be closed that way, and this is why: identity.profiles is keyed by an unlinkable pseudonym with no user_id and no foreign key, which is exactly what makes it pseudonymous and also what makes the database blind to the connection. Blobs are the same shape, and the MFA setup routes never load the user row at all. So PUT /user/identity recreated a name, a date of birth, a billing address and a VAT id on a subject the erasure had scrubbed, and nothing in the request path knew: the handler does not resolve the user, and Auth deliberately never reads the database. RequireLiveAccount is a lookup, applied to the five routes that persist subject-owned data and to nothing else. Read routes keep the stateless fast path that self-contained tokens exist to provide, and PUT /user/profile stays out of it because a second guard there would cost a round trip while hiding where the real invariant lives. It fails closed: a lookup error is a 401, because a database that cannot answer is not permission to write personal data back. Three existing gates had to learn the two new wrapper names, and each was worth the edit rather than a nuisance. The OWASP A01 gate keeps a guardComposes map precisely so a new guard's name cannot be trusted on its own; the NIST IA-11 gate checks the confirmation guard; the DPoP wiring gate checks both the route and the closure body. All three now cover the -Live pair, and the closure checks are what hold them to composing what they claim. Two register rationales cited server.go by line and this change moved both. They now point at the statements they always meant. The route gate is mutation-verified: swapping authedLive back to authed on one route fails it by name. It reads comment-free source, because the liveness meta-gate is right that a construct appearing only in a comment would otherwise satisfy the assertion. --- docs/compliance-register.json | 4 +- internal/middleware/liveaccount.go | 59 ++++++++++ internal/middleware/liveaccount_test.go | 125 ++++++++++++++++++++++ internal/server/server.go | 23 +++- tests/compliance/nist_800_53_test.go | 6 +- tests/compliance/owasp_top10_2025_test.go | 13 ++- tests/spec/dpop_route_wiring_test.go | 10 +- tests/spec/live_account_routes_test.go | 95 ++++++++++++++++ 8 files changed, 325 insertions(+), 10 deletions(-) create mode 100644 internal/middleware/liveaccount.go create mode 100644 internal/middleware/liveaccount_test.go create mode 100644 tests/spec/live_account_routes_test.go diff --git a/docs/compliance-register.json b/docs/compliance-register.json index 0f80c335..316ee9d4 100644 --- a/docs/compliance-register.json +++ b/docs/compliance-register.json @@ -178,7 +178,7 @@ "accepted_by": "Project maintainer (vault42)", "accepted_on": "2026-08-18", "security_md": "Register-scoped. docs/security.md owns the AR-nn namespace and defines a different risk under this number; the two lists are deliberately disjoint.", - "rationale": "internal/server/server.go:546 mounts GET /auth/verify-email, and internal/handler/auth.go:298 mutates state on it by marking the address verified. ASVS V3.5.3 requires either a non-safe method or strict Sec-Fetch-* validation.\n\nThis entry previously said no Sec-Fetch-* header is read anywhere in the tree, and that was false: cmd/bridge/proxy.go:373 reads all three. docs/COMPLIANCE.md recorded the correction and it never propagated back here, which is how a register sentence outlives the fact it rests on. The accurate statement is narrower and still decides the row: no route served by the vault itself validates fetch metadata, and adding it here would not help, because the rule only has teeth if it rejects requests that carry no fetch metadata at all -- which is every non-browser client, including the curl in docs/api.md and both shipped SDKs.\n\nThe method flip is the honest remedy and is a breaking change to a published API: @vault42/vue ships the GET in its built output, and a pinned older SDK would start receiving 405. It is timed with an SDK major rather than dropped into a patch release.", + "rationale": "internal/server/server.go:559 mounts GET /auth/verify-email, and internal/handler/auth.go:298 mutates state on it by marking the address verified. ASVS V3.5.3 requires either a non-safe method or strict Sec-Fetch-* validation.\n\nThis entry previously said no Sec-Fetch-* header is read anywhere in the tree, and that was false: cmd/bridge/proxy.go:373 reads all three. docs/COMPLIANCE.md recorded the correction and it never propagated back here, which is how a register sentence outlives the fact it rests on. The accurate statement is narrower and still decides the row: no route served by the vault itself validates fetch metadata, and adding it here would not help, because the rule only has teeth if it rejects requests that carry no fetch metadata at all -- which is every non-browser client, including the curl in docs/api.md and both shipped SDKs.\n\nThe method flip is the honest remedy and is a breaking change to a published API: @vault42/vue ships the GET in its built output, and a pinned older SDK would start receiving 405. It is timed with an SDK major rather than dropped into a patch release.", "compensating_control": "The token is single-use and consumed atomically by a get-and-delete -- Redis GETDEL, Postgres DELETE ... RETURNING, or the in-memory equivalent -- so a replayed or prefetched link verifies at most once, proven across replicas by the multi-replica end-to-end suite. The token is 256 bits from crypto/rand, stored hashed, and expires in 24 hours. The effect is bounded to marking an address the operator already sent mail to as verified: it confers no session and issues no token. The route is rate limited to 10 per hour per source address, and Referrer-Policy: no-referrer is set globally.", "residual_risk": "A security scanner that executes JavaScript performs the verification without the user acting. The precision matters and the earlier wording did not have it: the link in the email is the SPA route /verify-email, not the API route, so a plain prefetcher or link scanner that follows the URL receives index.html and changes nothing. The mutation happens only when the page's script runs and calls the API itself, which a headless renderer of the SafeLinks class does. Anything fetching /auth/verify-email directly also mutates, and that URL is published as a copyable curl in docs/api.md.\n\nThe consequence is not cosmetic. email_verified gates password login and is one half of the test deciding whether a federated identity may attach to an existing local account, so the realistic harm is an attacker registering with somebody else's address and that person's mail scanner completing the verification. A user whose token a scanner consumed also sees \"link expired\" while being, in fact, verified. Nothing else is reachable through the route.", "revisit_when": "Any additional state change is added behind a GET route, or the verification link starts conferring a session rather than only marking an address verified.", @@ -190,7 +190,7 @@ "accepted_by": "Project maintainer (vault42)", "accepted_on": "2026-08-18", "security_md": "Register-scoped. docs/security.md owns the AR-nn namespace and defines a different risk under this number; the two lists are deliberately disjoint.", - "rationale": "ASVS V3.5.4 wants separate applications on separate hostnames so the same-origin policy separates them. When VAULT_SERVE_FRONTEND is on, internal/server/server.go:847 mounts the embedded SPA at the catch-all route of the same server that answers /auth, /user and /client, so the two share an origin by construction and the policy separates nothing.", + "rationale": "ASVS V3.5.4 wants separate applications on separate hostnames so the same-origin policy separates them. When VAULT_SERVE_FRONTEND is on, internal/server/server.go:861 mounts the embedded SPA at the catch-all route of the same server that answers /auth, /user and /client, so the two share an origin by construction and the policy separates nothing.", "compensating_control": "VAULT_SERVE_FRONTEND is off by default (internal/config/config.go:505) and the chart ships serveFrontend: false, so the single-origin topology is opt-in; the honeypot profile is the one place that forces it on. The Content-Security-Policy applied when the frontend is served names script-src 'self' with no unsafe-inline, so even in the shared-origin shape a script the SPA did not author does not execute.\n\nThe chart offers the separated topology rather than shipping it, and the earlier wording did not make that distinction: charts/vault/templates/frontend.yaml deploys the SPA as its own nginx workload, but frontend.enabled is false by default and the template carries no hostname of its own. The separate hostname comes from ingress.hosts, which the operator supplies. So the preferred topology is available and configured, not default.", "residual_risk": "An operator who enables the embedded frontend gives up the origin boundary between the SPA and the API, so an XSS in the SPA runs on the API's origin.", "revisit_when": "The embedded frontend becomes the default, or the SPA gains a surface that renders content it did not author.", diff --git a/internal/middleware/liveaccount.go b/internal/middleware/liveaccount.go new file mode 100644 index 00000000..b16a81d7 --- /dev/null +++ b/internal/middleware/liveaccount.go @@ -0,0 +1,59 @@ +package middleware + +import ( + "net/http" + + "github.com/42-v/vault42/internal/httputil" + "github.com/42-v/vault42/internal/repository" +) + +// RequireLiveAccount refuses a request whose subject has been erased. +// +// An access token outlives the erasure that invalidated it. Auth validates the +// signature, issuer, audience and token type and never reads the database, which +// is the whole point of a self-contained token and is a trade docs/security.md +// makes deliberately: revocation is bounded by the access-token lifetime, not +// immediate. AR-5 states that for roles. +// +// Erasure is the case that trade does not cover. A stale role grants no more +// than it did when the token was minted; a write reaches into storage the +// erasure has already scrubbed and puts personal data back. The subject asked +// for it to be gone, the operator answered that it was, and neither is true +// afterwards. So this is not a general account-state check bolted onto every +// route -- it is applied to the handful of routes that persist subject-owned +// data, and nowhere else. +// +// Which routes: the ones that write and cannot be guarded in SQL. +// PUT /user/profile is not among them, because auth.users can state the rule +// itself and does -- UserRepo.Update carries AND deleted = FALSE. identity +// profiles cannot: identity.profiles is keyed by an unlinkable pseudonym with no +// user_id and no foreign key, which is what makes it pseudonymous and also what +// makes the database unable to see the connection. Blobs and the MFA setup +// routes are the same shape. For those, the only thing that can know is a +// lookup, so a lookup is what this does. +// +// The cost is one indexed primary-key read on a write path that is already +// doing storage work. Read routes are untouched, and the stateless fast path +// that self-contained tokens exist to provide is unchanged. +// +// Failure is closed. A lookup error is answered 401 rather than passed through: +// this guard exists because the alternative is writing personal data back onto +// an erased subject, and a database that cannot answer is not permission to do +// that. +func RequireLiveAccount(users repository.UserRepository) func(http.Handler) http.Handler { + return func(next http.Handler) http.Handler { + return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + claims := GetClaims(r.Context()) + if claims == nil { + httputil.WriteError(w, http.StatusUnauthorized, "unauthorized") + return + } + user, err := users.GetByID(r.Context(), claims.Subject) + if err != nil || user == nil || user.Deleted { + httputil.WriteError(w, http.StatusUnauthorized, "unauthorized") + return + } + next.ServeHTTP(w, r) + }) + } +} diff --git a/internal/middleware/liveaccount_test.go b/internal/middleware/liveaccount_test.go new file mode 100644 index 00000000..e607f0de --- /dev/null +++ b/internal/middleware/liveaccount_test.go @@ -0,0 +1,125 @@ +package middleware + +import ( + "context" + "errors" + "net/http" + "net/http/httptest" + "testing" + + vaultcrypto "github.com/42-v/vault42/internal/crypto" + vjwt "github.com/42-v/vault42/internal/jwt" + "github.com/42-v/vault42/internal/model" + "github.com/42-v/vault42/tests/mocks" +) + +// The routes behind this guard write personal data that an erasure has already +// scrubbed, into stores the database cannot check for itself. +// +// identity.profiles is keyed by an unlinkable pseudonym -- no user_id, no +// foreign key -- which is exactly what makes it pseudonymous and also what makes +// SQL unable to see that the subject is gone. PUT /user/identity therefore +// recreated a name, a date of birth, a billing address and a VAT id on a +// tombstoned subject, and the handler never resolved the user at all, so nothing +// in the request path knew. +// +// PUT /user/profile is deliberately not behind this: auth.users states the rule +// in its own UPDATE. A guard that duplicates one the database already enforces +// costs a round trip and hides where the real invariant lives. + +func liveAccountRequest(t *testing.T, subject string) *http.Request { + t.Helper() + req := httptest.NewRequest(http.MethodPut, "/user/identity", nil) + ctx := context.WithValue(req.Context(), ClaimsKey, &vaultcrypto.VaultClaims{RegisteredClaims: vjwt.RegisteredClaims{Subject: subject}}) + return req.WithContext(ctx) +} + +func runLiveAccount(t *testing.T, users *mocks.MockUserRepo, req *http.Request) (int, bool) { + t.Helper() + var reached bool + h := RequireLiveAccount(users)(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + reached = true + w.WriteHeader(http.StatusOK) + })) + rec := httptest.NewRecorder() + h.ServeHTTP(rec, req) + return rec.Code, reached +} + +func TestRequireLiveAccount_RefusesAnErasedSubject(t *testing.T) { + users := &mocks.MockUserRepo{ + GetByIDFn: func(_ context.Context, id string) (*model.User, error) { + // What SoftDeleteScrub leaves: the row survives so foreign keys stay + // valid, with the personal columns cleared and the tombstone set. + return &model.User{ID: id, Email: "deleted-" + id + "@deleted.invalid", Deleted: true}, nil + }, + } + code, reached := runLiveAccount(t, users, liveAccountRequest(t, "erased-user")) + if code != http.StatusUnauthorized { + t.Errorf("status = %d, want 401", code) + } + if reached { + t.Error("the handler ran for an erased subject. The status code is not the point " + + "on its own: what matters is that the write never happens.") + } +} + +func TestRequireLiveAccount_AllowsALiveSubject(t *testing.T) { + users := &mocks.MockUserRepo{ + GetByIDFn: func(_ context.Context, id string) (*model.User, error) { + return &model.User{ID: id, Email: "alice@example.com"}, nil + }, + } + code, reached := runLiveAccount(t, users, liveAccountRequest(t, "user-1")) + if code != http.StatusOK || !reached { + t.Fatalf("a live subject was refused: status = %d, reached = %v", code, reached) + } +} + +// Fail closed. A database that cannot answer is not permission to write personal +// data back onto a subject that may have been erased. +func TestRequireLiveAccount_FailsClosed(t *testing.T) { + cases := []struct { + name string + repo *mocks.MockUserRepo + }{ + {"lookup error", &mocks.MockUserRepo{ + GetByIDFn: func(_ context.Context, _ string) (*model.User, error) { + return nil, errors.New("connection refused") + }, + }}, + {"no such user", &mocks.MockUserRepo{ + GetByIDFn: func(_ context.Context, _ string) (*model.User, error) { + return nil, nil + }, + }}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + code, reached := runLiveAccount(t, tc.repo, liveAccountRequest(t, "user-1")) + if code != http.StatusUnauthorized || reached { + t.Fatalf("status = %d, reached = %v; want 401 and no handler", code, reached) + } + }) + } +} + +// No claims means the guard is mounted somewhere Auth is not, which is a wiring +// mistake. It must refuse rather than dereference nil. +func TestRequireLiveAccount_RefusesWithoutClaims(t *testing.T) { + var looked bool + users := &mocks.MockUserRepo{ + GetByIDFn: func(_ context.Context, _ string) (*model.User, error) { + looked = true + return &model.User{}, nil + }, + } + req := httptest.NewRequest(http.MethodPut, "/user/identity", nil) + code, reached := runLiveAccount(t, users, req) + if code != http.StatusUnauthorized || reached { + t.Fatalf("status = %d, reached = %v; want 401 and no handler", code, reached) + } + if looked { + t.Error("looked a subject up with no claims to name one") + } +} diff --git a/internal/server/server.go b/internal/server/server.go index c533a7bf..abd6d210 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -465,6 +465,19 @@ func (s *Server) setupRoutes() *http.ServeMux { confirmed := func(h http.HandlerFunc) http.Handler { return authMw(fingerprintMw(dpopWrap(confirmMw(h)))) } + // Routes that persist subject-owned data and cannot state the erasure rule + // in SQL. PUT /user/profile is deliberately absent: auth.users carries + // AND deleted = FALSE on its own UPDATE, so the database refuses it without + // being asked. identity.profiles is keyed by an unlinkable pseudonym with no + // user_id, blobs the same, and the MFA setup routes never load the user at + // all -- for those a lookup is the only thing that can know. + liveMw := middleware.RequireLiveAccount(d.Users) + authedLive := func(h http.HandlerFunc) http.Handler { + return authMw(fingerprintMw(dpopWrap(liveMw(h)))) + } + confirmedLive := func(h http.HandlerFunc) http.Handler { + return authMw(fingerprintMw(dpopWrap(confirmMw(liveMw(h))))) + } // Rate limiting middleware factories rlEnabled := cfg.RateLimitEnabled @@ -631,7 +644,7 @@ func (s *Server) setupRoutes() *http.ServeMux { mux.Handle("GET /auth/2fa/status", authed(mfaHandler.Status)) // 2FA — TOTP (sensitive ops require confirmation) - mux.Handle("POST /auth/2fa/totp/setup", confirmed(totpHandler.Setup)) + mux.Handle("POST /auth/2fa/totp/setup", confirmedLive(totpHandler.Setup)) mux.Handle("POST /auth/2fa/totp/verify", totpRL(authedChallenge(totpHandler.Verify))) mux.Handle("DELETE /auth/2fa/totp", confirmed(totpHandler.Disable)) @@ -660,7 +673,7 @@ func (s *Server) setupRoutes() *http.ServeMux { mux.Handle("DELETE /auth/2fa/webauthn/credentials/{id}", confirmed(webauthnHandler.DeleteCredential)) // 2FA — Backup codes (sensitive) - mux.Handle("POST /auth/2fa/backup-codes", confirmed(backupCodeHandler.Generate)) + mux.Handle("POST /auth/2fa/backup-codes", confirmedLive(backupCodeHandler.Generate)) mux.Handle("POST /auth/2fa/backup-code/verify", totpRL(authedChallenge(backupCodeHandler.Verify))) // 2FA — Email OTP (fallback when no TOTP/WebAuthn configured) @@ -684,7 +697,7 @@ func (s *Server) setupRoutes() *http.ServeMux { Name: "identitywrite", Limit: 10, Window: time.Minute, KeyFunc: middleware.IPRateLimitKey, }, rlEnabled) mux.Handle("GET /user/identity", identityReadRL(authed(identityHandler.Get))) - mux.Handle("PUT /user/identity", identityWriteRL(authed(identityHandler.Put))) + mux.Handle("PUT /user/identity", identityWriteRL(authedLive(identityHandler.Put))) mux.Handle("DELETE /user/identity", authMw(fingerprintMw(dpopWrap(confirmMw(confirmRL(http.HandlerFunc(identityHandler.Delete))))))) // Withdrawal must be no harder than granting (Art. 7(3)), so this carries // the read rate limit and no confirmation step — unlike identity deletion. @@ -706,11 +719,11 @@ func (s *Server) setupRoutes() *http.ServeMux { blobReadRL := middleware.RateLimit(d.Cache, middleware.RateLimitConfig{ Name: "blobread", Limit: 30, Window: time.Minute, KeyFunc: middleware.IPRateLimitKey, }, rlEnabled) - mux.Handle("POST /user/blobs", blobUploadRL(authed(blobHandler.Upload))) + mux.Handle("POST /user/blobs", blobUploadRL(authedLive(blobHandler.Upload))) mux.Handle("GET /user/blobs", blobReadRL(authed(blobHandler.List))) mux.Handle("GET /user/blobs/{id}", blobReadRL(authed(blobHandler.Download))) mux.Handle("DELETE /user/blobs/{id}", authMw(fingerprintMw(dpopWrap(confirmMw(confirmRL(http.HandlerFunc(blobHandler.Delete))))))) - mux.Handle("PUT /user/blobs/named/{name}", blobUploadRL(authed(blobHandler.UploadNamed))) + mux.Handle("PUT /user/blobs/named/{name}", blobUploadRL(authedLive(blobHandler.UploadNamed))) mux.Handle("GET /user/blobs/named/{name}", blobReadRL(authed(blobHandler.DownloadNamed))) mux.Handle("DELETE /user/blobs/named/{name}", authMw(fingerprintMw(dpopWrap(confirmMw(confirmRL(http.HandlerFunc(blobHandler.DeleteNamed))))))) } diff --git a/tests/compliance/nist_800_53_test.go b/tests/compliance/nist_800_53_test.go index 77fac0b1..6b6d205b 100644 --- a/tests/compliance/nist_800_53_test.go +++ b/tests/compliance/nist_800_53_test.go @@ -323,7 +323,11 @@ func TestNIST80053_IA_11_SensitiveChangesRequireRecentConfirmation(t *testing.T) t.Errorf("IA-11: route %q is no longer registered; re-derive this assertion", route) continue } - if !strings.Contains(wiring, "confirmed(") { + // Either confirming guard. confirmedLive is confirmed plus a check that + // the account has not been erased, so it is strictly stronger here; what + // IA-11 asks is that recent re-authentication was required, and both do + // that by composing confirmMw. + if !strings.Contains(wiring, "confirmed(") && !strings.Contains(wiring, "confirmedLive(") { t.Errorf("IA-11: %s changes an authentication factor without requiring recent re-authentication; it is wired as %s", route, wiring) } } diff --git a/tests/compliance/owasp_top10_2025_test.go b/tests/compliance/owasp_top10_2025_test.go index 2765d064..26971776 100644 --- a/tests/compliance/owasp_top10_2025_test.go +++ b/tests/compliance/owasp_top10_2025_test.go @@ -109,7 +109,10 @@ var publicRoutePrefixes = []string{ // reviewer has to confirm the closure really authenticates before the route // scan will accept it. TestOWASP_A01_2025_GuardClosuresReallyAuthenticate keeps // that confirmation executable rather than trusting this list. -var authGuards = []string{"authed(", "authMw(", "authedChallenge(", "confirmed(", "docRead(", "docWrite("} +var authGuards = []string{ + "authed(", "authMw(", "authedChallenge(", "confirmed(", "docRead(", "docWrite(", + "authedLive(", "confirmedLive(", +} // guardComposes names, per guard, the authentication middleware that guard is // entitled to compose — and only that one. @@ -132,6 +135,14 @@ var guardComposes = map[string]string{ "authedChallenge": "challengeMw(", "docRead": "authMw(", "docWrite": "authMw(", + // The -Live pair are the plain guards plus a lookup that refuses a subject + // whose account has been erased. They compose the same authentication + // middleware, which is what this map is asserting; the extra check is held + // separately by tests/spec/live_account_routes_test.go, because "is it + // authenticated" and "is the account still live" are different questions and + // a gate that conflated them would pass a route that had lost either. + "authedLive": "authMw(", + "confirmedLive": "authMw(", } func isDeclaredPublic(path string) bool { diff --git a/tests/spec/dpop_route_wiring_test.go b/tests/spec/dpop_route_wiring_test.go index 5ca63050..a2788459 100644 --- a/tests/spec/dpop_route_wiring_test.go +++ b/tests/spec/dpop_route_wiring_test.go @@ -44,11 +44,19 @@ var dpopWrapperIdents = []string{ "confirmed", "docRead", "docWrite", + // The -Live pair are the plain guards plus a lookup refusing an erased + // subject. They apply dpopWrap exactly as their counterparts do, and the + // closure check below is what holds them to it. + "authedLive", + "confirmedLive", } // routeBuilderClosures are the local helpers whose whole job is to build a // middleware chain for a family of routes. Each must apply dpopWrap. -var routeBuilderClosures = []string{"authed", "authedChallenge", "confirmed", "docRead", "docWrite"} +var routeBuilderClosures = []string{ + "authed", "authedChallenge", "confirmed", "docRead", "docWrite", + "authedLive", "confirmedLive", +} // dpopExemptRoutes are the registrations that legitimately run no DPoP // middleware. Each needs a reason, because adding a route here is how the gate diff --git a/tests/spec/live_account_routes_test.go b/tests/spec/live_account_routes_test.go new file mode 100644 index 00000000..b3425954 --- /dev/null +++ b/tests/spec/live_account_routes_test.go @@ -0,0 +1,95 @@ +package spec_test + +import ( + "path/filepath" + "regexp" + "strings" + "testing" +) + +// The routes that persist subject-owned data have to keep their erasure guard. +// +// An access token outlives the erasure that invalidated it, so a write reaching +// storage the erasure already scrubbed puts personal data back onto a subject +// who asked for it to be gone. auth.users can refuse that itself -- UserRepo's +// UPDATE carries AND deleted = FALSE -- but the other stores cannot. +// identity.profiles is keyed by an unlinkable pseudonym with no user_id and no +// foreign key, which is what makes it pseudonymous and also what makes SQL blind +// to the connection; blobs are the same shape; and the MFA setup routes never +// load the user row at all. +// +// For those the only thing that can know is a lookup, and the only thing holding +// the lookup in place is one wrapper name at one call site. Swapping authedLive +// back to authed compiles, serves, and passes every handler test, because no +// handler test goes through the router. This gate reads the routing table so +// that edit fails the build instead. +// +// The tests are read-only. They never write to the source tree. + +var serverRouting = filepath.Join("internal", "server", "server.go") + +// guardedRoutes are the routes that write subject-owned data into a store the +// database cannot check. The value is the wrapper each must be mounted with. +// +// PUT /user/profile is deliberately absent: auth.users states the rule in its +// own UPDATE, and a second guard there would cost a round trip while hiding +// where the real invariant lives. +var guardedRoutes = map[string]string{ + "PUT /user/identity": "authedLive", + "POST /user/blobs": "authedLive", + "PUT /user/blobs/named/{name}": "authedLive", + "POST /auth/2fa/totp/setup": "confirmedLive", + "POST /auth/2fa/backup-codes": "confirmedLive", +} + +func TestSubjectWritingRoutesRequireALiveAccount(t *testing.T) { + src := commentFreeSource(t, filepath.Join(repoRoot(t), serverRouting)) + + var checked int + for route, wrapper := range guardedRoutes { + // mux.Handle("", ...) -- the rest of the line holds the wrappers. + pattern := regexp.MustCompile(`mux\.Handle\("` + regexp.QuoteMeta(route) + `",([^\n]*)`) + m := pattern.FindStringSubmatch(src) + if m == nil { + t.Errorf("no mux.Handle for %q in %s. If the route moved or was renamed, move "+ + "this gate with it rather than deleting the entry: what it holds is that an "+ + "erased subject cannot write personal data back.", route, serverRouting) + continue + } + checked++ + if !strings.Contains(m[1], wrapper) { + t.Errorf("%q is not mounted with %s:\n\t%s\n"+ + "Without it the handler runs for a subject whose data has already been "+ + "erased, and writes it back into a store that cannot tell.", + route, wrapper, strings.TrimSpace(m[1])) + } + } + + if checked == 0 { + t.Fatalf("this gate matched no routes at all in %s, so it proved nothing", serverRouting) + } +} + +// The wrappers have to actually contain the guard. Naming one authedLive while +// it wraps the same chain as authed would satisfy the test above and nothing else. +func TestTheLiveWrappersActuallyCarryTheGuard(t *testing.T) { + src := commentFreeSource(t, filepath.Join(repoRoot(t), serverRouting)) + + for _, name := range []string{"authedLive", "confirmedLive"} { + decl := regexp.MustCompile(name + ` := func\(h http\.HandlerFunc\) http\.Handler \{\s*\n\s*return ([^\n]*)`) + m := decl.FindStringSubmatch(src) + if m == nil { + t.Errorf("no %s wrapper declared in %s", name, serverRouting) + continue + } + if !strings.Contains(m[1], "liveMw") { + t.Errorf("%s does not apply liveMw:\n\t%s\nThe name says the account is checked; "+ + "the body has to be what checks it.", name, strings.TrimSpace(m[1])) + } + } + + if !strings.Contains(src, "middleware.RequireLiveAccount(") { + t.Error("server.go no longer constructs middleware.RequireLiveAccount, so whatever " + + "liveMw now is, it is not the erasure guard") + } +} From 9c206fc72dea73754aef19ecb058c1533e9a5211 Mon Sep 17 00:00:00 2001 From: 42-v <210031137+42-v@users.noreply.github.com> Date: Thu, 27 Aug 2026 00:35:18 +0200 Subject: [PATCH 2/2] docs(badges): recount after rebasing onto main --- README.md | 2 +- docs/badges.json | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 75198c96..6acec89b 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ Vault42 issues its own tokens and is an OAuth2 *client* of other providers. It i | ![Go](https://img.shields.io/badge/Go-1.26.6-00ADD8?style=flat&logo=go&logoColor=white) | ![Vue](https://img.shields.io/badge/Vue-3.5.41-4FC08D?style=flat&logo=vuedotjs&logoColor=white) | ![.NET](https://img.shields.io/badge/.NET-10.0-512BD4?style=flat&logo=dotnet&logoColor=white) | ![License](https://img.shields.io/badge/License-MIT-155724?style=flat&labelColor=000) | | ![Go Tests](https://img.shields.io/badge/Tests-4910-155724?style=flat&labelColor=000) | ![Vue Tests](https://img.shields.io/badge/Tests-1305-155724?style=flat&labelColor=000) | ![C# Tests](https://img.shields.io/badge/Tests-264-155724?style=flat&labelColor=000) | ![Total](https://img.shields.io/badge/Total-6479_tests-155724?style=flat&labelColor=000) | | ![Go Coverage](https://img.shields.io/badge/Coverage-100.00%25_reachable-155724?style=flat&labelColor=000) | ![Vue Coverage](https://img.shields.io/badge/Coverage-99.76%25-155724?style=flat&labelColor=000) | ![C# Coverage](https://img.shields.io/badge/Coverage-100.00%25-155724?style=flat&labelColor=000) | ![Locales](https://img.shields.io/badge/Locales-38-555?style=flat&labelColor=000) | -| ![Go Lines](https://img.shields.io/badge/Lines-48278-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) | +| ![Go Lines](https://img.shields.io/badge/Lines-48350-555?style=flat&labelColor=000) | ![Vue Lines](https://img.shields.io/badge/Lines-6798-555?style=flat&labelColor=000) | ![C# Lines](https://img.shields.io/badge/Lines-2435-555?style=flat&labelColor=000) | ![Standards](https://img.shields.io/badge/Standards-11-555?style=flat&labelColor=000) | | ![Go Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![Vue Deps](https://img.shields.io/badge/Deps-3-555?style=flat&labelColor=000) | ![C# Deps](https://img.shields.io/badge/Deps-6-555?style=flat&labelColor=000) | ![Requirements](https://img.shields.io/badge/Requirements-456-555?style=flat&labelColor=000) | | ![Go Transitive Deps](https://img.shields.io/badge/Transitive-15-555?style=flat&labelColor=000) | ![Vue Transitive Deps](https://img.shields.io/badge/Transitive-95-555?style=flat&labelColor=000) | ![C# Transitive Deps](https://img.shields.io/badge/Transitive-26-555?style=flat&labelColor=000) | ![Total Deps](https://img.shields.io/badge/Deps-148_total-555?style=flat&labelColor=000) | diff --git a/docs/badges.json b/docs/badges.json index 43170d6e..b4de5df4 100644 --- a/docs/badges.json +++ b/docs/badges.json @@ -8,9 +8,9 @@ "reachableCoverage": "100.00%", "reachableCoverageNum": 100.00, "packages": 43, - "goFiles": 192, - "goLines": 48278, - "testFiles": 932, + "goFiles": 193, + "goLines": 48350, + "testFiles": 934, "directDeps": 3, "transitiveDeps": 15, "totalTests": 6479, @@ -20,7 +20,7 @@ "tests": 4910, "coverage": "100.00% of reachable", "coverageNum": 100.00, - "lines": 48278, + "lines": 48350, "deps": 3, "transitiveDeps": 15 },