From fa0e77657a8496a02e69592f1bfd57574cb132ad Mon Sep 17 00:00:00 2001 From: Harry Phan Date: Sat, 20 Jun 2026 19:35:44 +0700 Subject: [PATCH] fix(api): verify hosted delegate identity to close owner-auth IDOR (#23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .gitignore | 3 + .../0008_hosted_delegate_binding.sql | 19 +++ apps/api/src/worker.test.ts | 113 ++++++++++++++++++ apps/api/src/worker.ts | 78 +++++++++++- 4 files changed, 208 insertions(+), 5 deletions(-) create mode 100644 apps/api/migrations/0008_hosted_delegate_binding.sql diff --git a/.gitignore b/.gitignore index 3b92893..d949f65 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,9 @@ coverage/ .env .env.* !.env.example +# Cloudflare Worker local secrets (HARBOR_*, MEMWAL_*, import token) — never commit. +.dev.vars +**/.dev.vars runs/ artifacts/ *.tsbuildinfo diff --git a/apps/api/migrations/0008_hosted_delegate_binding.sql b/apps/api/migrations/0008_hosted_delegate_binding.sql new file mode 100644 index 0000000..18bf14e --- /dev/null +++ b/apps/api/migrations/0008_hosted_delegate_binding.sql @@ -0,0 +1,19 @@ +-- Hosted delegate identity binding (#23): make the self-asserted x-memwal-account-id +-- header non-spoofable. +-- +-- The hosted Worker has no signup/session, so before this change a caller could +-- claim ANY owner id just by setting a header, and the namespace artifact-edit route +-- (`POST /api/namespaces/:ns/artifact-edit`) authorized writes on a raw string match +-- against that header. We now bind each hosted owner to the FIRST delegate secret we +-- see (trust-on-first-use) and verify every later owner-scoped mutation against the +-- stored hash, so an attacker who guesses a victim's owner id but lacks the bound +-- delegate secret is rejected. +-- +-- secret_hash is sha256(owner_id || ':' || delegate_secret) — salted by owner so the +-- raw delegate secret is never stored. created_at/last_seen_at are advisory. +CREATE TABLE IF NOT EXISTS contextmem_hosted_delegates ( + owner_id TEXT PRIMARY KEY, + secret_hash TEXT NOT NULL, + created_at TEXT NOT NULL, + last_seen_at TEXT NOT NULL +); diff --git a/apps/api/src/worker.test.ts b/apps/api/src/worker.test.ts index c1800ba..e3b615b 100644 --- a/apps/api/src/worker.test.ts +++ b/apps/api/src/worker.test.ts @@ -749,6 +749,105 @@ describe("ContextMeM hosted namespace Worker", () => { restoreFetch(); } }); + + it("binds a hosted owner to its first delegate secret and blocks a spoofed x-memwal-account-id (#23)", async () => { + const env = createTestEnv(); + const { handleWorkerRequest, CloudflareNamespaceStore } = await worker(); + const namespace = "web:owned-by-acct-a.com"; + + const imported = await handleWorkerRequest( + new Request("https://contextmem.test/api/namespaces/import", { + method: "POST", + headers: { authorization: "Bearer import-secret", "content-type": "application/json" }, + body: JSON.stringify({ + namespace, + visibility: "private", + ownerId: "hosted:acct-a", + target: "https://demo-product.wal.app/", + sourceRunId: "run_fixture", + manifest: { target: "https://demo-product.wal.app/", pages: [] }, + files: [ + { path: "/site/page.md", contentType: "text/markdown; charset=utf-8", encoding: "utf8", content: "# original" } + ] + }) + }), + env + ); + expect(imported.status).toBe(201); + + const editUrl = `https://contextmem.test/api/namespaces/${encodeURIComponent(namespace)}/artifact-edit`; + const editBody = JSON.stringify({ path: "/site/page.md", content: "# edited by owner" }); + + // First sighting of acct-a with its delegate secret binds (trust-on-first-use) and succeeds. + const ownerEdit = await handleWorkerRequest( + new Request(editUrl, { + method: "POST", + headers: { + "content-type": "application/json", + "x-memwal-account-id": "acct-a", + "x-memwal-authorization": "Bearer delegate-secret-aaaa" + }, + body: editBody + }), + env + ); + expect(ownerEdit.status).toBe(200); + const stored = await new CloudflareNamespaceStore(env).readArtifact(namespace, "/site/page.md"); + expect(stored?.content).toContain("edited by owner"); + + // An attacker who guesses owner acct-a but presents a DIFFERENT secret is rejected. + const spoofedEdit = await handleWorkerRequest( + new Request(editUrl, { + method: "POST", + headers: { + "content-type": "application/json", + "x-memwal-account-id": "acct-a", + "x-memwal-authorization": "Bearer attacker-secret-zzzz" + }, + body: JSON.stringify({ path: "/site/page.md", content: "# hijacked" }) + }), + env + ); + expect(spoofedEdit.status).toBe(403); + const afterSpoof = await new CloudflareNamespaceStore(env).readArtifact(namespace, "/site/page.md"); + expect(afterSpoof?.content).not.toContain("hijacked"); + }); + + it("scopes schedules/alerts to a delegated owner and fails closed without one (#23)", async () => { + const env = createTestEnv(); + const { handleWorkerRequest } = await worker(); + + // Seed two owners' schedules via the trusted server-to-server proxy (import token). + for (const ownerId of ["hosted:acct-a", "hosted:acct-b"]) { + const created = await handleWorkerRequest( + new Request("https://contextmem.test/api/schedules", { + method: "POST", + headers: { authorization: "Bearer import-secret", "content-type": "application/json" }, + body: JSON.stringify({ ownerId, target: "https://demo-product.wal.app/", intervalHours: 1 }) + }), + env + ); + expect(created.status).toBe(201); + } + + // Trusted proxy delegating an explicit owner sees only that owner's rows. + const scopedToA = await handleWorkerRequest( + new Request("https://contextmem.test/api/schedules?ownerId=hosted%3Aacct-a", { + headers: { authorization: "Bearer import-secret" } + }), + env + ); + const scopedBody = (await scopedToA.json()) as { schedules: Array<{ ownerId?: string }> }; + expect(scopedBody.schedules).toHaveLength(1); + + // Import token but NO owner delegated → fail closed to an empty list (no anonymous dump). + const noOwner = await handleWorkerRequest( + new Request("https://contextmem.test/api/schedules", { headers: { authorization: "Bearer import-secret" } }), + env + ); + const noOwnerBody = (await noOwner.json()) as { schedules: unknown[] }; + expect(noOwnerBody.schedules).toHaveLength(0); + }); }); async function importFixtureNamespace(env: WorkerEnv, visibility: "private" | "public", options: Record = {}) { @@ -854,6 +953,7 @@ class MemoryD1Database { scheduleRuns = new Map>(); alerts = new Map>(); webhookDeliveries = new Map>(); + hostedDelegates = new Map>(); artifacts: Array> = []; prepare(query: string) { @@ -889,6 +989,9 @@ class MemoryD1Statement { if (query.includes("from contextmem_schedules") && query.includes("where id = ?")) { return (this.db.schedules.get(String(this.values[0])) as T | undefined) ?? null; } + if (query.includes("from contextmem_hosted_delegates") && query.includes("where owner_id = ?")) { + return (this.db.hostedDelegates.get(String(this.values[0])) as T | undefined) ?? null; + } if (query.includes("from contextmem_namespaces") && query.includes("where namespace = ?")) { return (this.db.namespaces.get(String(this.values[0])) as T | undefined) ?? null; } @@ -1067,6 +1170,16 @@ class MemoryD1Statement { } else if (query.startsWith("insert into contextmem_alerts")) { const [id, owner_id, schedule_id, namespace, target, title, message, diff_json, created_at] = this.values; this.db.alerts.set(String(id), { id, owner_id, schedule_id, namespace, target, title, message, diff_json, read_at: null, created_at }); + } else if (query.startsWith("insert into contextmem_hosted_delegates")) { + const [owner_id, secret_hash, created_at, last_seen_at] = this.values; + // ON CONFLICT(owner_id) DO NOTHING — keep the first binding. + if (!this.db.hostedDelegates.has(String(owner_id))) { + this.db.hostedDelegates.set(String(owner_id), { owner_id, secret_hash, created_at, last_seen_at }); + } + } else if (query.startsWith("update contextmem_hosted_delegates")) { + const [last_seen_at, owner_id] = this.values; + const row = this.db.hostedDelegates.get(String(owner_id)); + if (row) row.last_seen_at = last_seen_at; } else if (query.startsWith("insert into contextmem_webhook_deliveries")) { const [id, alert_id, webhook_url, created_at, updated_at] = this.values; this.db.webhookDeliveries.set(String(id), { id, alert_id, webhook_url, status: "queued", attempts: 0, created_at, updated_at }); diff --git a/apps/api/src/worker.ts b/apps/api/src/worker.ts index cf352e9..6674ab4 100644 --- a/apps/api/src/worker.ts +++ b/apps/api/src/worker.ts @@ -1117,15 +1117,19 @@ async function updateNamespaceArtifact(request: Request, env: WorkerEnv, namespa .first<{ namespace: string; owner_id: string; current_version_id: string; byte_length: number; manifest_json: string | null }>(); if (!row) return json({ error: "Namespace not found." }, 404); - const providedOwner = request.headers.get("x-memwal-account-id") ?? new URL(request.url).searchParams.get("ownerId") ?? ""; + // Authorize the edit against a VERIFIED owner, never a self-asserted header (#23): + // - the trusted server-to-server proxy (holds the import secret), or + // - a hosted delegate whose secret matches its trust-on-first-use binding, or + // - the demo owner derived from the caller's own request. const callerDemoOwner = demoOwnerId(request); const importAuth = requireImportAuthorization(request, env); + const verifiedOwner = await resolveDelegateOwner(request, env); const ownerMatch = importAuth.ok || - (providedOwner && providedOwner === row.owner_id) || + (verifiedOwner !== undefined && verifiedOwner === row.owner_id) || (row.owner_id.startsWith("demo:") && row.owner_id === callerDemoOwner); if (!ownerMatch) { - return json({ error: "You do not own this namespace. Pass x-memwal-account-id or the import token." }, 403); + return json({ error: "You do not own this namespace. Pass a verified x-memwal-account-id delegate or the import token." }, 403); } const store = new CloudflareNamespaceStore(env); @@ -1517,6 +1521,65 @@ function normalizeDelegateSecret(value: string): string { return value.replace(/^Bearer\s+/i, "").trim(); } +// Verify the hosted delegate identity so a spoofed `x-memwal-account-id` header +// cannot impersonate another owner (#23). The Worker has no signup/session, so we +// bind each hosted owner to the FIRST delegate secret we see (trust-on-first-use) +// and reject any later request that presents a different secret for that owner. +// Returns the canonical hosted owner id when the delegate is verified (or freshly +// bound), or undefined when no delegate is present or the secret does not match. +// The salted hash (sha256(ownerId:secret)) means the raw delegate secret is never +// stored. +async function resolveDelegateOwner(request: Request, env: WorkerEnv): Promise { + const auth = readHostedRunAuth(request); + if (!auth) return undefined; + const rawSecret = request.headers.get("x-memwal-authorization") ?? request.headers.get("x-memwal-bearer") ?? ""; + const secret = normalizeDelegateSecret(rawSecret); + if (secret.length < 12) return undefined; + const secretHash = await sha256Hex(`${auth.ownerId}:${secret}`); + const now = new Date().toISOString(); + const existing = await env.CONTEXTMEM_DB.prepare( + `SELECT secret_hash FROM contextmem_hosted_delegates WHERE owner_id = ?` + ) + .bind(auth.ownerId) + .first<{ secret_hash: string }>(); + if (!existing) { + // Trust on first use: bind this owner to the delegate secret presented now. + await env.CONTEXTMEM_DB.prepare( + `INSERT INTO contextmem_hosted_delegates (owner_id, secret_hash, created_at, last_seen_at) VALUES (?, ?, ?, ?) + ON CONFLICT(owner_id) DO NOTHING` + ) + .bind(auth.ownerId, secretHash, now, now) + .run(); + // Re-read so a concurrent first-write race resolves to whichever secret won. + const bound = await env.CONTEXTMEM_DB.prepare( + `SELECT secret_hash FROM contextmem_hosted_delegates WHERE owner_id = ?` + ) + .bind(auth.ownerId) + .first<{ secret_hash: string }>(); + return bound && constantTimeEqual(bound.secret_hash, secretHash) ? auth.ownerId : undefined; + } + if (!constantTimeEqual(existing.secret_hash, secretHash)) return undefined; + await env.CONTEXTMEM_DB.prepare(`UPDATE contextmem_hosted_delegates SET last_seen_at = ? WHERE owner_id = ?`) + .bind(now, auth.ownerId) + .run(); + return auth.ownerId; +} + +// Resolve the authoritative owner for an owner-scoped route. A trusted +// server-to-server caller (one that holds the import secret — i.e. the local Fastify +// proxy, which has already verified its own user's session) may delegate an explicit +// owner id. Everyone else is scoped to their verified hosted delegate. The `?ownerId=` +// param is NEVER trusted on its own. Returns undefined when no trustworthy owner is +// available, so callers fail closed instead of falling back to a shared "anonymous" +// bucket that would leak cross-owner rows. +async function resolveScopedOwner(request: Request, env: WorkerEnv): Promise { + if (requireImportAuthorization(request, env).ok) { + const delegated = new URL(request.url).searchParams.get("ownerId")?.trim(); + if (delegated) return delegated; + } + return resolveDelegateOwner(request, env); +} + async function createDemoExtraction(request: Request, env: WorkerEnv, ctx: WorkerExecutionContext): Promise { const input = demoExtractionCreateSchema.parse(await request.json().catch(() => ({}))); const target = validatePublicDemoTarget(input.sample ? demoSampleTarget(env) : input.target ?? demoSampleTarget(env)); @@ -1701,7 +1764,10 @@ async function createSchedule(request: Request, env: WorkerEnv): Promise { - const ownerId = new URL(request.url).searchParams.get("ownerId")?.trim() || "anonymous"; + // Fail closed: only a verified owner (trusted-proxy delegation or a verified hosted + // delegate) is scoped here; never default to a shared "anonymous" bucket (#23). + const ownerId = await resolveScopedOwner(request, env); + if (!ownerId) return json({ schedules: [] }); const result = await env.CONTEXTMEM_DB.prepare( `SELECT id, owner_id, namespace, target, interval_hours, webhook_url, webhook_secret, active, last_run_at, next_run_at, created_at, updated_at FROM contextmem_schedules @@ -1739,7 +1805,9 @@ async function updateSchedule(request: Request, env: WorkerEnv, scheduleId: stri } async function listAlerts(request: Request, env: WorkerEnv): Promise { - const ownerId = new URL(request.url).searchParams.get("ownerId")?.trim() || "anonymous"; + // Fail closed: see listSchedules — never default to a shared "anonymous" bucket (#23). + const ownerId = await resolveScopedOwner(request, env); + if (!ownerId) return json({ alerts: [] }); const result = await env.CONTEXTMEM_DB.prepare( `SELECT id, owner_id, schedule_id, namespace, target, title, message, diff_json, read_at, created_at FROM contextmem_alerts