Repository navigation
Google-only sign-in on hosted deployments - #29
Merged
Merged
Conversation
Hosted deployments now require a real Google identity to mint API keys. API keys become publish-only: they can upload and read but can no longer sign in or mint further keys, so a key leaked from CI config or a dotfile cannot take over the account. Hosted mode fails closed. If OAuth were ever misconfigured, key creation returns 503 rather than silently accepting an API key -- a config regression must not quietly remove the identity requirement. Self-hosted deployments keep key-paste sign-in via AUTH_MODE, since a self-hoster may have no IdP available.
Left to guess, an agent invents flags that don't exist, publishes a revision as a second draft and strands the reviewer on a dead link, reaches for an external stylesheet the HTML policy rejects, or hands back a /tmp path instead of a URL. This encodes the behaviour that works. Every command, flag, error code, and exit code is verified against the real CLI and live API rather than written from the docs, including two behaviours the docs do not state: --new rewrites the local path->draft mapping so the previous draft is orphaned, and an upload with unchanged content still creates a new version because dedupe is by idempotency key and not by content hash.
The old favicon was leftover WebHost branding: a blue #6ea8fe tile with a W squiggle, unrelated to the product palette. And only the dashboard served any icon at all -- the CONTENT origin, where humans actually open published drafts, 404'd every icon path, so every draft showed a blank globe in the tab. That is the surface reviewers see most. Content origin serves /favicon.ico and /favicon.svg at the root. It cannot inject a <link> into a document because pages are served byte-for-byte, so root-level icons are the only correct mechanism: a browser requests /favicon.ico on its own when a document declares no icon. The ICO is embedded as base64 rather than read from R2 or an assets binding, which would widen the untrusted-content origin's surface to serve 5 KB. Dashboard gains a multi-resolution ICO (16/32/48), an opaque 180px apple-touch-icon -- iOS composites a transparent one onto WHITE -- PWA icons, a maskable icon, a manifest, and a theme-color matching --bg so mobile chrome does not flash white against the dark UI. Favicon geometry is pixel-aligned for 16px rendering: bars and gaps are even units, so edges land on whole device pixels instead of antialiasing to mush. Verified numerically -- zero antialiased interior pixels, equal bar lengths, accent ratio 0.50, padding balanced 3/3. All derived assets are generated by scripts/gen-icons.sh from two SVG sources of truth, with scripts/verify_icons.py asserting 36 properties including that the base64 embedded in the worker decodes to byte-identical bytes and its 16px frame matches a fresh SVG render. That check exists because a hand-rolled embed silently corrupted the base64 -- it compiled clean and would have served a broken image.
Adds the favicon mark as a reusable inline-SVG Logo component in the nav
lockup, plus a README/docs variant, so the mark is one thing in three
places instead of three drifting copies.
FIXES THE EMBEDDED VIEWER (found while verifying this). The content
worker sent `frame-ancestors 'self'` with a comment claiming it allowed
the dashboard preview iframe. It does not: 'self' is the CONTENT origin,
while the iframe is embedded from the DASHBOARD origin, so every browser
refused the frame and the preview rendered as Chrome's blank subframe
error page. The comment asserted the opposite of the behaviour, which is
how it survived review. frame-ancestors is now built per request from a
new DASHBOARD_ORIGIN var, naming the embedding origin explicitly, and
falls back to 'none' rather than 'self' when unset -- an unconfigured
untrusted-content origin should refuse all framing, not guess.
Proven with a differential in a real browser: /health and /d/:id on the
SAME origin and worker, only the latter carrying frame-ancestors. Both
fetched fine, /health committed and rendered its JSON, /d/:id stayed on
the error glyph. That isolates the CSP header as the cause rather than a
network or 5xx failure, which a blank frame alone cannot distinguish.
Two defects in my own first cut, both caught by inspecting the rendered
result rather than the source:
* The nav link announced "agentdraft agentdraft" to a screen reader --
the SVG's aria-label duplicated the adjacent wordmark. The mark is now
decorative by default and takes a label only when it stands alone.
* The badge was invisible: a #0a0a0a tile on a #0a0a0a nav with a
#2a2a2a edge, and a 1-unit stroke in a 32-unit viewBox renders at
~0.7px. Tile now sits a step above the background with a 1.5-unit
light stroke, confirmed via computed style, not by eye.
The nav CSS selector was also narrowed to .brand__word: a bare
`.nav .brand span` would match the new wordmark wrapper and paint the
whole word accent instead of just "draft".
README logo keeps transparent corners so it reads on GitHub in both light
and dark themes, and verify_icons.py now asserts that plus the accent
colour and the README reference, so a rename cannot silently break the
render. 41/41 icon checks pass.
The preview iframe stayed refused after frame-ancestors was fixed, because Cross-Origin-Resource-Policy: same-origin blocks cross-origin embedding INDEPENDENTLY of the CSP. Two gates, one visible symptom -- an identical blank frame -- which is exactly how the first fix passed verification while prod stayed broken. Now same-site rather than removed: both origins are tapetide.com subdomains, so the dashboard can embed a draft while an arbitrary third-party site still cannot hotlink one. COOP stays same-origin; it governs window.opener, not framing. Verified against the direct workers.dev origin (the custom domain served a stale zone-cached copy for ~1 min after deploy, which is its own trap when confirming a header change).
Every published draft was readable by anyone holding its URL, forever, with
no revocation. is_public already existed in the schema and the content
worker already enforced it -- but NOTHING could ever set it. This makes it
settable, from the CLI and the dashboard, per draft and per account.
HOW A PRIVATE DRAFT IS VIEWED. The shared URL is unchanged and just opens
for its owner. The content origin stays cookie-free (it serves
attacker-controlled HTML, so a session must never be readable there), so it
cannot authorize anyone: it 302s to the dashboard, which holds the session,
verifies OWNERSHIP, and streams the same bytes. Signed-out viewers land on
sign-in and return to the exact document via a validated ?next=.
Owner-only, not any-signed-in-user. Widening access later is easy;
un-leaking a draft is not.
NOT RETROACTIVE. The account default applies to NEW drafts only. Flipping a
preference must never silently break links already shared with reviewers, so
existing drafts change only via an explicit bulk endpoint, which reports how
many rows it touched.
IDS: 12 -> 22 chars base36 (62 -> 113 bits). Measured first: 62 bits was
already not enumerable (~7.5 years to a first hit at 100k req/s against 100k
drafts, before rate limiting), so this is defence in depth and NOT the
privacy mechanism -- a leaked URL is readable at any entropy. Existing
12-char ids keep working and are not migrated; people already shared them.
The ?next= open-redirect guard is the risky part, so it is allow-list based
and tested (31 cases: scheme-relative, backslash, encoded, control chars).
MUTATION TESTED, which changed what I believe: deleting the startsWith("//")
check leaves all 31 tests GREEN, because a string starting "//" can never
start "/private/d/" either -- the allow-list is the load-bearing defence and
the "//" check is redundant. Deleting the allow-list fails 2 tests at once.
That finding is recorded in the test file so nobody relaxes the allow-list
believing the "//" check protects them. Wired into CI.
Verified against live prod, not just locally: anonymous request to a private
draft returns 302 with zero content bytes; following it anonymously lands on
sign-in with next= preserved; owner reads 200 with content; a DIFFERENT
account's key gets 404 (not 403 -- existence is not disclosed); no-auth gets
401; /raw and /v/N redirect too; ?v=abc 400s and ?v=999 404s; flipping back
to public restores anonymous reads (revocable both directions, no stale
302 cached). Migration confirmed non-destructive: 95 drafts before and
after, 0 privatised. Bulk call touched only my 4 drafts, the other account's
90 untouched.
Completes the CLI half of private drafts. Backend shipped in 3b946b0 but nothing in the CLI could reach it. agentdraft upload plan.md --private create it owner-only agentdraft visibility private plan.md by file, id, or url agentdraft visibility public --all every existing draft agentdraft visibility private --default default for NEW drafts only agentdraft list now carries a VISIBILITY column agentdraft whoami shows what the next upload will be TWO DEFECTS FOUND WHILE WIRING THIS, both in existing code: 1. `fetch` passed an EMPTY api key, so the new private-draft fallback could never have authenticated. It now loads stored credentials, while still working unauthenticated for public drafts. 2. `fetch` followed redirects blindly. A private draft 302s to the dashboard sign-in page, so fetch would have written an HTML LOGIN PAGE to the output file and exited 0 — a silent wrong-content success, the worst failure shape available. It now refuses to follow, detects the hop, and retries through the authenticated API endpoint. UploadRequest.Public is a *bool, not a bool: with omitempty a plain false is dropped from the JSON, which would publish a draft the user asked to keep private. The pointer keeps "unspecified" (use the account default) distinct from "explicitly public". Re-uploading never changes an existing draft's visibility. Silently flipping a shared link on the next upload would be a nasty surprise; that is what the visibility command is for. VISIBILITY column added to list because a private URL is byte-identical to a public one — without it the only way to tell them apart is opening the link in a logged-out browser, which is the mistake the column prevents. Tested against live prod: --private upload then anonymous read = 302 with zero content bytes; owner fetch returns the real document (marker present, no login-page words); fetch with NO credentials exits 1 with a clear message and writes 0 bytes rather than a login page; visibility resolves by id, file path, and full url; bad verb, missing target, and --all/--default together all rejected. Added draftPathRe unit tests covering legacy 12-char and current 22-char ids, MUTATION TESTED: loosening the floor to {1,32} fails the reject suite immediately, so the guard is proven rather than assumed.
Completes private drafts. Backend landed in 3b946b0 and the CLI in cdd0859; until now nothing in the dashboard could change visibility. Per-draft toggle sits directly under the title on the draft detail page, because visibility decides who can read the document and belongs next to its identity rather than buried in settings. New /settings/privacy page carries the account default plus the explicit bulk action, linked from the nav. DESIGN POINTS WORTH KEEPING: The DESIRED state is submitted in a hidden field, never a toggle computed from the rendered value. A toggle derived from stale page data flips the WRONG way when two tabs are open or when the value changed via the CLI since the page loaded. Both actions return without redirecting, so SvelteKit re-runs load() and the page re-reads the authoritative value from the API instead of trusting what was just sent. Bulk confirmation names the REAL count ("Make 3 public drafts private?") and the buttons disable at zero. A generic "are you sure?" is easy to click past; the number is what makes the consequence land. The count comes from the API, and its response reports rows actually changed so a no-op says so rather than claiming success. DEFECT FOUND BY EXERCISING THE ACTION, NOT THE PAGE: I first named the settings action `default`. SvelteKit RESERVES that name for the unnamed default action and rejects an explicit ?/default with a 500 -- so the button was dead. Rendering the page returned a healthy 200 the whole time and svelte-check was clean, so nothing short of POSTing the form surfaced it. Renamed to setdefault. Verified against live prod by driving each form action: setdefault persists and /api/me confirms it; the page then re-renders with the new label; bulk private moves 3 -> 0 public and bulk public restores it; per-draft toggle flips cas3z3e5k05r and anonymous reads go 200 -> 302 -> 200 across the change. Rendered counts matched the API exactly (3 public / 0 private). Account default and all draft visibilities restored to their original state afterwards.
Hitesh-Sisara
added a commit
that referenced
this pull request
Aug 9, 2026
) Found while running the adversarial suite against prod after the domain-ref fix. Section 6 minted a limited key with `POST /api/api-keys` using $KEY as a manage credential — but #29 (Google-only hosted auth) added requireHumanSession() to the key-management routes. In hosted mode, NO bearer key can list or mint keys; that now requires a Google-signed-in dashboard session. So the suite could no longer complete its own setup against a hosted deployment, and its assertions encoded the self-hosted contract only. The suite now detects auth_mode from GET /api/config and branches: hosted -> assert the machine key is REFUSED from LIST and MINT (403), which is the actual security property #29 introduced: a key leaked from CI or a dotfile can publish but can never escalate to take over the account. Also assert it can still upload. self-hosted -> the original path: manage key mints a limited key, proves the limited key cannot list/mint, then revokes and proves it dead. AUTH_MODE is printed in the run banner so the operator sees which contract was asserted. Verified live against prod (hosted): full run 197/197, exit 0. Re-confirmed the hosted branch executes (AUTH_MODE=hosted; machine key -> 403 on LIST and MINT) rather than being silently skipped. The suite's own rate-limit guard (100 uploads/hour/key) fires on a second run inside the hour, exactly as documented — use a fresh key per run.
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
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.
Hosted deployments now require a verified Google identity to mint API keys.
What changed
AUTH_MODE, since a self-hoster may have no IdP.OAuth client
agentdraft was initially wired to the shared tapetide-mcp client (project
key-range-490704-p9), whose allow-list holds onlyhttps://mcp.tapetide.com/callback— so every sign-in failedredirect_uri_mismatch. It now uses a dedicated client in projecttapetide-authwithhttps://app.agentdraft.tapetide.com/auth/google/callbackregistered. Client id/secret live only asagentdraft-apiWorker secrets; the dashboard is a BFF and never sees the secret.Verified against live prod
Error 400: redirect_uri_mismatch)evil.example.com->redirect_uri_mismatch, so ACCEPTED is meaningfulinvalid_grantnotinvalid_client, proving client auth succeeds and only the fake code was rejectedE_BAD_STATE(CSRF guard live)scope=openid email profileNot covered
The final browser leg (real Google consent -> callback -> session cookie) needs a human at the account chooser. Every mechanical leg either side of it is verified above.