fix(api): verify hosted delegate identity to close owner-auth IDOR (#23)#30
Merged
Conversation
Hosted owner identity was fully self-asserted: the namespace artifact-edit route (POST /api/namespaces/:ns/artifact-edit, no dispatch auth gate) granted writes on a raw string match against a caller-supplied x-memwal-account-id / ?ownerId, and listSchedules/listAlerts defaulted to a shared "anonymous" bucket. Anyone who knew a victim's owner id (derivable from a public Sui/MemWal account id) could edit their namespace artifacts. - Add resolveDelegateOwner(): the Worker has no signup/session, so bind each hosted owner to the first delegate secret seen (trust-on-first-use) and verify every later owner-scoped mutation against the salted hash. A spoofed account-id without the bound secret is rejected. Raw secret never stored. - Add resolveScopedOwner(): a trusted server-to-server caller (holds the import secret — the local Fastify proxy that already verified its user) may delegate an explicit owner; everyone else is scoped to their verified delegate. ?ownerId is never trusted on its own. - updateNamespaceArtifact: authorize against the verified owner, not the self-asserted header. - listSchedules/listAlerts: resolve via resolveScopedOwner and fail closed to an empty list instead of dumping the "anonymous" bucket. - migration 0008: contextmem_hosted_delegates binding table. - .gitignore: ignore .dev.vars so Worker local secrets are never committed. - tests: cross-owner artifact-edit is denied; the bound owner can edit; schedules scope to a delegated owner and fail closed without one. Remaining #23 (separate, larger): a verified ctx_ session resolveOwner over contextmem_sessions, delegate->account association, per-plan quota replacing the hardcoded unlimited, and demo-owner confinement across all owner-scoped routes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the client-reachable owner-authorization IDOR in the hosted Worker. Scoped to making authorization trustworthy — the larger #23 identity overhaul (verified sessions, per-plan quota, demo confinement) is called out as remaining work below.
The hole
Hosted owner identity was entirely self-asserted:
POST /api/namespaces/:ns/artifact-edit→updateNamespaceArtifacthas no dispatch auth gate; it authorized writes on a raw string match against a caller-suppliedx-memwal-account-id/?ownerId=. Anyone who knew a victim's owner id (derivable from a public Sui/MemWal account id) could overwrite their namespace artifacts.listSchedules/listAlertstrusted?ownerId=and defaulted to a shared"anonymous"bucket.Root cause: the hosted Worker has no signup/session, so a caller could claim any owner id just by setting a header.
The fix (trust-on-first-use delegate binding)
resolveDelegateOwner()— binds each hosted owner to the first delegate secret seen (TOFU) in a newcontextmem_hosted_delegatestable, then verifies every later owner-scoped mutation against the saltedsha256(ownerId:secret)hash (constant-time). A spoofedx-memwal-account-idwithout the bound secret is rejected. The raw secret is never stored.resolveScopedOwner()— a trusted server-to-server caller (holds the import secret, i.e. the local Fastify proxy that already verified its own user) may delegate an explicit owner id; everyone else is scoped to their verified delegate.?ownerId=is never trusted on its own.updateNamespaceArtifactauthorizes against the verified owner, not the self-asserted header.listSchedules/listAlertsresolve viaresolveScopedOwnerand fail closed to an empty list instead of dumping the"anonymous"bucket.0008adds thecontextmem_hosted_delegatesbinding table..gitignorenow ignores.dev.vars(Cloudflare Worker local secrets —HARBOR_*,MEMWAL_*, import token) so they can't be committed; this entry was missing onmain.Testing
[]without one.npx vitest run— 76/76 pass.tsc --noEmitclean forapps/api.Out of scope (remaining #23, larger follow-up)
A verified
ctx_-sessionresolveOwnerovercontextmem_sessions, associating delegate import with an account row, replacing the hardcodedquota.unlimitedwith a per-plan lookup, and confining demo owners — across all owner-scoped routes (/api/namespaces,/api/extractions,/api/usage). Those routes are currently import-token gated (trusted proxy), so they are not a client-reachable hole today.TOFU caveat
Trust-on-first-use means if an attacker presents a victim's never-before-seen owner id before the victim ever does, the attacker's secret becomes the binding (a DoS / pre-registration hijack window). The full session-based identity above removes this; documented here as a known limitation of the scoped fix.