Recording → editable steps (HarnessRouter), fail-closed auth, and org invitations/RBAC - #403
Merged
Merged
Conversation
Builds the "Convert" half of Phase 2 (docs/CURSOR_HANDOFF.md): upload the
raw trace of one demonstrated workflow, have a purpose-configured
HarnessRouter agent propose a typed step plan, review and edit it in the
existing WorkflowEditor, then publish it through the existing
POST /api/workflows path.
The proposal is a proposal. It is validated against @ghost/core/schema/step
before it is stored, it is never executed, and publishing re-validates it
exactly as it would a hand-authored definition — the AI pre-fills the
editor and nothing more. Capture (remote browser vs. extension) remains the
open Phase 2 decision; this sits behind whatever produces the trace file.
Recording gains a compile lifecycle (RecordingCompileStatus) separate from
its capture status, so a STOPPED recording can be recompiled after a failure
without pretending the capture needs to happen again.
Notes on the HarnessRouter integration:
- HR_API_KEY is read server-side only; every call is made from a route
handler, and file bytes are relayed through our own authenticated routes
rather than handing the browser an API-key-protected download_url.
- POST /v1/files returns the upload id as `id`, not `file_id` — the session
files API uses `file_id` for a different id entirely. Reading the wrong
key does not throw: it yields undefined, the input_file block is still
accepted, and the agent runs with no attachment and reports only that it
cannot find the file. uploadFile now fails loudly and a test pins it.
- The SSE parser dispatches on data.type (there are no `event:` lines) and
ignores unknown types.
- startResponse returns once it has the recovery identifiers rather than
holding the connection open; closing it does not stop the run.
- Compile status is reconciled lazily, when the detail route or stream is
polled, so a closed tab cannot strand a finished run as RUNNING forever.
- An empty steps array is a real answer ("no browser actions in this
trace"), reported as such alongside the compiler's notes rather than as a
schema error about needing at least one step.
Verified end to end through the running app: upload, compile, streamed
progress, review, publish. The compiler correctly emitted semantic
role+name selectors and inserted an approval gate before a "Pay now" click.
Cross-tenant access to every recording route returns 404, the stream
reports notFound, and claiming another org's recording while publishing is
refused with 409. Org audit chain verifies intact across the run.
pnpm typecheck, pnpm test (272 tests), and pnpm build all pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three ways the current setup fails silently — the app boots, sign-in works, and nothing looks wrong: **A published session secret.** `.env.example` ships a working `AUTH_SECRET` so `cp .env.example .env` yields a running stack (the same deliberate trade-off as `GHOST_SESSION_KEY`). It is in the repository, so anyone can forge a session for any user in any organization. Production now refuses to start on it, on an unset value, or on anything under 32 characters. Outside production it only warns — breaking `pnpm dev` over a dev secret would just push people back to the published one. `next build` is exempt. It runs with NODE_ENV=production and imports every route module to collect page data, so asserting there makes a real secret a prerequisite for *compiling*, including in CI, which has no business holding one. Building is not serving; the check still runs when the module is first imported to handle a request. **A dev provider gated on one variable.** The passwordless "any email" provider was registered whenever `NODE_ENV !== "production"` — a single variable that is routinely unset and that nobody treats as a security boundary. It now also requires the instance to be loopback-only (via AUTH_URL) and honours an explicit off switch, so forgetting any one signal fails safe. An unparseable or absent URL counts as exposed. **A matcher that silently drifts.** `/audit` and `/recordings` were both missing from the middleware matcher. Nothing leaked — `(app)/layout.tsx` redirects when there is no session, and that is what actually stops an unauthenticated render — but two guards disagreeing stays invisible until the day the layout stops being the thing that saves you. Both are added, and `middleware.test.ts` now derives the expected list from the filesystem so a new page without an entry fails the suite. Verified by removing an entry and watching it fail. The matcher has to stay an inline literal: Next.js analyses `config.matcher` statically and rejects an imported identifier, so the test reads and parses the source rather than importing it (which would also drag the Auth.js runtime into a node test). Session lifetime drops from Auth.js's 30-day default to 12 hours, overridable via GHOST_SESSION_MAX_AGE_SECONDS. First of four agreed auth workstreams; invites/RBAC, MFA/SSO, and per-tenant identity to the runtime agent are still to come. pnpm typecheck, pnpm test (297 tests), and pnpm build all pass. Local dev sign-in re-verified after the change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Second of the four auth workstreams. Until now `ensureUserOrg` created a
single-member organization on first sign-in and there was no way to add a
second person — which meant `requireSeparateApprover` (four-eyes approval)
could never fire, because an org never contained anyone else to approve.
**Invitations.** Admin-issued, hashed like an agent credential (only the
SHA-256 digest is stored; the link is shown once and is unrecoverable).
Two differences from an agent token, because this grants standing access to
a tenant rather than a scoped capability:
- it expires (7 days). An invite sitting in an inbox for a year is a way
into the organization long after whoever sent it stopped meaning it;
- holding the token is not sufficient. Acceptance also requires the
signed-in user's email to match the invited address, so a forwarded or
leaked link cannot be redeemed by whoever finds it.
Every rejection — expired, revoked, spent, wrong recipient, never existed —
returns the same 404 and the same message, so a URL cannot be used to probe
whether an organization ever invited a given address.
**Member management.** List, change role, remove. Both mutations are
guarded by `wouldOrphanOrganization`: an org with no OWNER cannot invite,
cannot change roles and cannot revoke anything, and no ordinary member can
revive it. Demoting the last owner bricks it exactly as thoroughly as
deleting them, so both paths check.
**Auth.js edge split.** Claiming an invitation during sign-in put
`node:crypto` (invitation hashing, the audit hash chain) into `auth.ts`'s
import graph, and `middleware.ts` imports `auth.ts` — so the whole graph got
bundled for the Edge runtime, where `node:crypto` does not exist, and the
build failed outright. Middleware now builds from a minimal `auth.config.ts`
instead, the documented Auth.js v5 pattern: it only needs to answer "is
there a valid session token?", which under the JWT strategy is a signature
check with no database. Side benefit: the middleware bundle drops from
147 kB to 87 kB.
**One thing only end-to-end testing found.** A newcomer's invitation is
consumed by `ensureUserOrg` at sign-in, so they then arrive at the accept
page holding a correctly-spent link — and were told "that invitation is not
valid for this account" despite having just successfully joined. Accept now
recognises that case and reports membership instead. It discloses nothing:
the caller is provably already inside the organization.
Known limitation, surfaced rather than hidden: session `orgId` is stamped
into the JWT at sign-in and there is no org switcher, so someone who
already belonged to another org must sign in again to see the new one. The
accept page says so.
Verified end to end in the browser: invite issued, a non-invitee redeeming
the link refused (and the invitation *not* burned by the failed attempt),
the invitee joined the shared workspace with no stray personal org, and a
MEMBER sees roles read-only with no invite or remove controls. Audit chain
records member.invited -> member.joined.
pnpm typecheck, pnpm test (319 tests), pnpm build all pass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mohabbis
changed the title
Owner
Author
Added: organization invitations and member management (workstream 2 of 4)Until now Invitations — admin-issued, hashed like an agent credential (only the SHA-256 digest stored; link shown once). Two deliberate differences from an agent token, because this grants standing access to a tenant rather than a scoped capability:
Every rejection (expired / revoked / spent / wrong recipient / never existed) returns the same 404 and the same message, so a URL can't be used to probe whether an org ever invited a given address. Member management — list, change role, remove. Both mutations guarded by Auth.js edge split. Claiming an invitation at sign-in put One bug only end-to-end testing found. A newcomer's invitation is consumed by Known limitation, surfaced not hidden: session Verified in the browser, not just in testsInvite issued → a non-invitee redeeming the link refused, and the invitation not burned by the failed attempt → the invitee joined the shared workspace with no stray personal org → a MEMBER sees roles read-only with no invite/remove controls. Audit chain records
Remaining agreed auth work: MFA/SSO, and per-tenant identity to the runtime agent. Capture (remote cloud browser vs. Chrome extension) is a genuine architectural fork — I'll bring a recommendation rather than guess. |
Third of the four auth workstreams. **TOTP implemented here rather than taken from a package.** It is roughly sixty lines of well-specified arithmetic, both RFCs publish test vectors, and it sits on the authentication path where an unaudited transitive dependency is a poor trade. `mfa.test.ts` checks it against all ten RFC 4226 HOTP vectors and all six RFC 6238 TOTP vectors, which is a stronger correctness argument than "a library did it" — a hand-rolled TOTP that is subtly wrong still produces six digits that change every thirty seconds and only fails when a real authenticator app disagrees. **Secrets are encrypted at rest** under their own key (`GHOST_MFA_KEY`, AES-256-GCM, user id bound in as associated data). It cannot share `GHOST_SESSION_KEY`: that one is deliberately absent from the web app so a web-side vulnerability cannot decrypt a captured browser session, and verifying a code is something the web app does on every sign-in. With no key configured, enrolment is refused rather than storing secrets in the clear. **Enrolment is two steps on purpose.** Beginning enrolment stores a secret but does not activate anything; 2FA turns on only once a code round-trips. Activating on step one would lock out anyone who opened the screen, never scanned the code, and closed the tab. Disabling requires a current code too — otherwise a stolen session alone is enough to strip the second factor. Recovery codes are single-use, digest-only, consumed atomically so a replay cannot race itself, and their use is audited. **Two things end-to-end testing caught that tests and typechecking did not:** 1. *MFA was enforced on pages but not on the API.* The matcher covered `/settings/:path*`, which does not match `/api/settings/...` — so `/api/*` was never matched at all and anyone holding a session cookie could read the same data by calling the API directly, skipping the challenge entirely. Verified before the fix: `/api/runs` returned data while `/dashboard` redirected. The matcher now covers the session-authenticated API surface, excluding `auth` (gating it prevents signing in), `agent` (bearer-credential authenticated, no session) and `mfa` (how the challenge is answered). API callers get 401/403 JSON rather than a redirect to an HTML page. 2. *The verification survived sign-out.* Bound to the user alone, the cookie minted when the factor was proved kept satisfying the gate across a sign-out and a fresh sign-in for its whole 12-hour life — so someone holding only the password could sign in on that machine and never be challenged, which is the precise thing a second factor exists to prevent. It is now bound to a per-sign-in `sid`, so a new sign-in invalidates it. Confirmed in the running app: 403 -> verify -> 200 -> sign out/in -> 403. The verified-this-session marker is a separate signed httpOnly cookie rather than a session claim, because Auth.js exposes a client-callable session update endpoint — anything the `jwt` callback accepts from an update is in effect client-settable, which is the wrong shape for a flag asserting a challenge was passed. Not included: enterprise SSO (SAML/OIDC). That needs an IdP integration and is a separate piece of work, not a variation on this one. pnpm typecheck, pnpm test (359 tests), pnpm build all pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two facts a deployer needs before pointing this at real systems, neither of which is visible from the code without going looking. `HR_API_KEY` is process-wide, so every organization's traces and compile sessions share one HarnessRouter Workspace. Ghost's own routes are org-scoped and refuse cross-tenant access — verified: detail, compile, continue, cancel and stream all 404, and claiming another org's recording while publishing is refused — but that is Ghost enforcing isolation on its own surface, not isolation inside HarnessRouter. Anyone holding the key can enumerate every tenant's sessions there directly. Per-organization credentials would be the fix and do not exist yet. And a trace is customer data: a recording of real work in real systems. The compiler is instructed to replace captured secrets with placeholders, but that is an instruction to a model, not an enforced guarantee, and the trace is uploaded whole regardless. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Added: TOTP two-factor authentication (workstream 3 of 4)TOTP implemented here rather than pulled from a package — ~60 lines of well-specified arithmetic on the authentication path, where an unaudited transitive dependency is a poor trade. Secrets are encrypted at rest under their own key ( Enrolment is two steps deliberately: beginning it stores a secret but activates nothing; 2FA turns on only once a code round-trips. Activating on step one would lock out anyone who opened the screen and closed the tab. Disabling also requires a current code — otherwise a stolen session alone strips the second factor. Two bugs end-to-end testing caught that tests and typechecking did not1. MFA was enforced on pages but not on the API. The matcher covered 2. The verification survived sign-out. Bound to the user alone, the cookie minted when the factor was proved kept satisfying the gate across sign-out and a fresh sign-in for its full 12h life — so someone holding only the password could sign in on that machine and never be challenged. That is precisely what a second factor exists to prevent. Now bound to a per-sign-in The verified-this-session marker is a separate signed httpOnly cookie rather than a session claim, because Auth.js exposes a client-callable session-update endpoint — anything the Not included: enterprise SSO (SAML/OIDC). That needs an IdP integration and is separate work, not a variation on this.
Workstream 4: not built, deliberately — and why
It does point at a real gap, which is now documented in The fix is per-organization HarnessRouter credentials. That's a product decision with real cost (each org needs its own Workspace) — worth deciding rather than guessing. |
The previous framing had this backwards. `.env.example` called HR_API_KEY "runtime agent execution", DEPLOY.md offered per-organization HarnessRouter credentials as the fix for shared tenancy, and the README described the convert path by naming the vendor. All of that reads as though customer workflows execute through HarnessRouter. They do not, and they must not. HarnessRouter is agent infrastructure used to build Ghost. It backs exactly one optional authoring convenience — recording compile, whose output is a proposal a human reviews and republishes through the normal workflow path. Nothing a customer executes touches it, and production is expected to leave the key unset. Per-organization workspaces were the wrong fix to reach for: they would have addressed the shared-Workspace problem and not the one that matters more, which is that a trace is customer data — bodies, headers, typed input — uploaded whole and unredacted to a third party the customer never contracted with. Buying a provisioning system for a dependency that should not be in the runtime is a worse outcome than removing it from the runtime. Documentation only. The code still requires the key for compile; extracting a provider-neutral WorkflowCompiler interface is the next change, and CURSOR_HANDOFF now says so rather than leaving the boundary implied. Also records the capture decision: Chrome extension for v1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 4, 2026
* ci: gate path-scoped jobs with if:, not workflow path filters Every cloud-only PR was permanently unmergeable. Branch protection requires fourteen Rust CI contexts; rust.yml carried `paths-ignore: cloud/**`, so on a cloud-only change the workflow never started, never created those check runs, and the PR sat at "Expected — Waiting for status" with nothing to click. mergeStateStatus BLOCKED, every check that did run green, no way forward except an admin override. PR #403 is the current example. A workflow skipped by a path filter creates no check runs. A *job* skipped by an `if:` condition does create one, concludes "skipped", and branch protection counts that as satisfied. So both workflows now always start, decide once in a `changes` job whether their tree was touched, and let every other job skip itself when it wasn't. `changes` reads the PR's file list from the API rather than cloning full history to diff it — this repo is large enough that fetch-depth: 0 on every job is a real cost. Push events skip the detection entirely and run everything, since only pull requests are gated by branch protection. An empty or unreadable file list falls through to running CI, so the failure mode is wasted minutes rather than an ungated merge. This also lets cloud's `build` become a required context for the first time: it now reports on Rust-only PRs (as skipped) instead of never appearing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: fix detection job working dir, and fail closed when it breaks Two defects in the previous commit, both caught by its own PR run. The cloud detection job inherited the workflow-level `working-directory: cloud` but never checks out, so bash could not start: "No such file or directory". rust.yml has no such default, which is why only one of the two failed. The worse one: `build` still reported "skipped" after that failure, and branch protection accepts skipped. A broken detection step would therefore have merged cloud changes with no CI at all — fail-open, the exact hazard this whole change exists to remove. Both workflows now run the gated jobs when detection itself fails. `!cancelled()` is required for a job to be considered at all once a dependency has failed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 4, 2026
#405) * ci: require one aggregate Rust context instead of fourteen matrix ones PR #404 moved path scoping into an `if:` so required checks would always report. That works for non-matrix jobs — Rustfmt, Version consistency and Frontend contract tests all reported "skipped" on #403 as intended — and does not work for the matrix ones. A matrix job skipped by an `if:` condition never expands its matrix, so its check run is created under the literal name "Check (${{ matrix.os }})" rather than "Check (ubuntu-latest)". Requiring the expanded names therefore leaves them waiting forever on a cloud-only PR: the same deadlock #404 set out to fix, one level down, which is why #403 was still BLOCKED afterwards. A non-matrix aggregator has a static name in every case. `rust-ci` needs the seven previously-required jobs, runs with `always()` so it still reports when they skipped, and fails if any of them failed or was cancelled. Required contexts become "Rust CI" and "build" — two instead of fifteen, and adding an OS to the matrix no longer means editing branch protection. `experimental` is deliberately not among the dependencies: it was not a required context before this change and this is not the commit that makes it one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * ci: fix invalid expression in the Rust CI aggregator GitHub expressions accept single-quoted string literals only. The aggregator used join(needs.*.result, ",") with double quotes, which is a workflow-file syntax error rather than a runtime one: the run fails to start, no jobs are created, and the PR shows no Rust checks at all — not a failure that points at the offending line. Replaced with the contains(needs.*.result, 'failure') pattern, which needs no string literal argument and is the more conventional spelling anyway. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 5, 2026
…oc reconciliation (#409) * feat(cloud): browser recording capture — Chrome extension + deterministic compile Priority 3. Ghost could execute and approve, but nobody could record — the only way to get a workflow into Ghost was to upload a trace file produced by some other tool. There was no capture at all, and `RecordingStatus.ACTIVE` was never set because nothing recorded. Per the capture decision in docs/ARCHITECTURE_DECISIONS.md §3, this ships a Chrome extension for v1, execution stays entirely server-side. ## The trace contract (`@ghost/core/recording/trace`) A Zod schema for what a recorder uploads: navigate/click/input/select/submit events, each carrying a `TraceTarget` with accessible role, name, test id and ordered CSS fallbacks. The defining property: **role and name are read at capture time**, off the live DOM while the element is still on screen — not inferred later from an opaque trace. Those are exactly the fields `resolveLocator` already prefers, so nothing downstream had to change. A secret is never captured, not encrypted or truncated — absent. `redacted: true` records that something was typed without recording what. This is the only place that redaction can be done honestly: a trace carrying the value plus a "please ignore this" flag has already leaked it (see the P0-2 finding in the architecture doc, about the previous design uploading traces whole to a third party). ## The compiler (`@ghost/core/recording/compile`) Trace to typed steps, deterministically. No model, no network, no configured compiler. Three things it does beyond translation: - collapses a run of keystrokes on one field into one `fill` with the final value - runs every produced step through `classifyStep` — the same deterministic classifier the worker consults at run time — and inserts an `approval` immediately before anything it would gate, so the authored workflow agrees with what execution will actually do - never carries a redacted value; a secret field becomes a `fill` marked `sensitive` with a placeholder, and a note tells the reviewer to set it This is what makes recording work with no compiler configured — the state production runs in per the HarnessRouter decision (§1). A model still has a job: naming steps, proposing gates beyond the obvious cases, flagging what it could not resolve. Deciding which element was clicked is not a judgement call and should never have been one. 24 tests, including a fixture shaped exactly like the extension's actual output (`roundtrip.test.ts`) — the seam most likely to rot, since the extension is untyped JS with nothing else to notice if its output drifts from what the compiler expects. ## Ingest (`lib/recording-ingest.ts`) Shared by the existing upload form and the extension, so upload limits, filename sanitisation and audit events cannot drift between the two paths. A structured Ghost trace compiles inline and lands `READY` in the same request; anything else (HAR, Playwright zip) is stored and left for whatever compiler is configured, unchanged from before. `POST /api/agent/recordings` is new: the extension's ingest point, on the already bearer-authenticated agent surface rather than the session-only upload route, since the extension runs at a different origin and cannot rely on the session cookie reaching it. `resolveAgentPrincipal` enforces the same second factor on its session fallback as every other agent route (see the P0-1 fix). Uploading only creates a proposal — nothing here publishes a workflow or executes anything; the human still reviews compiled steps in the editor and publishes through `POST /api/workflows`, which revalidates them. ## The extension (`apps/extension`) Manifest V3. `content.js` captures clicks, typing, selects, submits and SPA navigation, computing accessible name in roughly accname-spec order from label/aria-label/aria-labelledby/placeholder/value/text. Secret fields are detected by `type=password`, secret-shaped `autocomplete`, or a name/label/id matching a word list (password, otp, cvv, card number, ssn, ...) — and the value is never read for them, not merely withheld after reading. `background.js` buffers events locally and uploads only on Stop, via a revocable bearer token created in Ghost Settings. The popup is deliberately thin — start, stop, where to send it — because a second place to edit steps would be a second thing to keep in sync with the real editor. ## Validation 398 tests pass (up from 374 — 24 new), against a database migrated from zero. `pnpm typecheck` and `pnpm build` both clean with `HR_API_KEY` unset. Manually verified `ingestTrace` end-to-end against a real Postgres: a structured trace lands `compileStatus: READY` with steps persisted, in one request, no compiler configured. ## What is not yet covered `GET /api/recordings` gained a `take: 50` (P2-2 for this route) as an incidental fix while touching the file; the rest of that finding stands. Extension host permissions are `<all_urls>` for v1; scoping to allowlisted origins per organization is a natural follow-up once there is a customer to scope it for. No component tests for the extension itself — Manifest V3 content scripts have no test harness in this repo, so `roundtrip.test.ts` pinning its exact output shape is the coverage that exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(worker): make production container build and boot reliably The worker Dockerfile had never actually been built or run since it was written. Auditing what was genuinely left on the cloud roadmap surfaced two stacked bugs, neither caught by CI (`pnpm build` only bundles the worker with tsup at the workspace level — it never exercises the Dockerfile's own COPY list or executes the resulting dist/index.js): - The deps stage copied packages/core/package.json but not its prisma/ directory. @ghost/core's postinstall runs a bare `prisma generate`, which resolves the schema at the default ./prisma/schema.prisma path, so `pnpm install` failed inside the image before the build stage (which does copy the rest of packages/core) ever ran. The same gap silently dropped tsconfig.base.json, breaking apps/worker/tsconfig.json's `extends`. - Once building, the container crashed immediately on boot: bundling @ghost/core (via tsup's `noExternal: [/^@ghost\//]`) pulls in @prisma/client's generated CJS runtime, which dynamically requires native query-engine files — esbuild's CJS-to-ESM interop can't represent that and throws "Dynamic require of 'fs' is not supported" at the first call. Fixed by keeping @prisma/client external in tsup.config.ts and adding it as a direct dependency of @ghost/worker, since pnpm's strict linking won't resolve a transitive dep at the worker's own require path otherwise. Verified by rebuilding the image and running it against real Postgres/Redis until it logged its startup line rather than crashing. Added a CI step that builds and boot-smoke-tests the image on every PR so this class of bug can't ship invisibly again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(cloud): reconcile handoff and status documentation with current implementation CURSOR_HANDOFF.md described the WorkflowCompiler abstraction, four-eyes approval, and S3 artifact serving as open work, framing a stale roadmap that led to prioritizing already-shipped items. All three were fully implemented and tested by earlier PRs (#403, #406) whose changes never made it back into this file's "remaining work" section: - WorkflowCompiler: apps/web/src/lib/compiler/{types,index,harness-router-compiler}.ts already provides the interface, a swappable HarnessRouter adapter, and normalized error types. - Four-eyes approval: Membership/Role/Invitation plus Organization.requireSeparateApprover already enforce requester != approver server-side, covered by separation-of-duties.test.ts. - S3 artifact serving: packages/core/src/storage/artifacts.ts already has a working S3ArtifactStore with presigned URLs; only retention/cleanup is genuinely still open, called out as such. Also documents the worker container bugs found and fixed in the prior commit, and corrects the stale "239 tests" figure (actual full green run is 398) and the "typed step editor: remaining" status line (workflow-editor.tsx already covers it) in both this file and README.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 5, 2026
… opt-in Sentry (#410) * feat(cloud): browser recording capture — Chrome extension + deterministic compile Priority 3. Ghost could execute and approve, but nobody could record — the only way to get a workflow into Ghost was to upload a trace file produced by some other tool. There was no capture at all, and `RecordingStatus.ACTIVE` was never set because nothing recorded. Per the capture decision in docs/ARCHITECTURE_DECISIONS.md §3, this ships a Chrome extension for v1, execution stays entirely server-side. ## The trace contract (`@ghost/core/recording/trace`) A Zod schema for what a recorder uploads: navigate/click/input/select/submit events, each carrying a `TraceTarget` with accessible role, name, test id and ordered CSS fallbacks. The defining property: **role and name are read at capture time**, off the live DOM while the element is still on screen — not inferred later from an opaque trace. Those are exactly the fields `resolveLocator` already prefers, so nothing downstream had to change. A secret is never captured, not encrypted or truncated — absent. `redacted: true` records that something was typed without recording what. This is the only place that redaction can be done honestly: a trace carrying the value plus a "please ignore this" flag has already leaked it (see the P0-2 finding in the architecture doc, about the previous design uploading traces whole to a third party). ## The compiler (`@ghost/core/recording/compile`) Trace to typed steps, deterministically. No model, no network, no configured compiler. Three things it does beyond translation: - collapses a run of keystrokes on one field into one `fill` with the final value - runs every produced step through `classifyStep` — the same deterministic classifier the worker consults at run time — and inserts an `approval` immediately before anything it would gate, so the authored workflow agrees with what execution will actually do - never carries a redacted value; a secret field becomes a `fill` marked `sensitive` with a placeholder, and a note tells the reviewer to set it This is what makes recording work with no compiler configured — the state production runs in per the HarnessRouter decision (§1). A model still has a job: naming steps, proposing gates beyond the obvious cases, flagging what it could not resolve. Deciding which element was clicked is not a judgement call and should never have been one. 24 tests, including a fixture shaped exactly like the extension's actual output (`roundtrip.test.ts`) — the seam most likely to rot, since the extension is untyped JS with nothing else to notice if its output drifts from what the compiler expects. ## Ingest (`lib/recording-ingest.ts`) Shared by the existing upload form and the extension, so upload limits, filename sanitisation and audit events cannot drift between the two paths. A structured Ghost trace compiles inline and lands `READY` in the same request; anything else (HAR, Playwright zip) is stored and left for whatever compiler is configured, unchanged from before. `POST /api/agent/recordings` is new: the extension's ingest point, on the already bearer-authenticated agent surface rather than the session-only upload route, since the extension runs at a different origin and cannot rely on the session cookie reaching it. `resolveAgentPrincipal` enforces the same second factor on its session fallback as every other agent route (see the P0-1 fix). Uploading only creates a proposal — nothing here publishes a workflow or executes anything; the human still reviews compiled steps in the editor and publishes through `POST /api/workflows`, which revalidates them. ## The extension (`apps/extension`) Manifest V3. `content.js` captures clicks, typing, selects, submits and SPA navigation, computing accessible name in roughly accname-spec order from label/aria-label/aria-labelledby/placeholder/value/text. Secret fields are detected by `type=password`, secret-shaped `autocomplete`, or a name/label/id matching a word list (password, otp, cvv, card number, ssn, ...) — and the value is never read for them, not merely withheld after reading. `background.js` buffers events locally and uploads only on Stop, via a revocable bearer token created in Ghost Settings. The popup is deliberately thin — start, stop, where to send it — because a second place to edit steps would be a second thing to keep in sync with the real editor. ## Validation 398 tests pass (up from 374 — 24 new), against a database migrated from zero. `pnpm typecheck` and `pnpm build` both clean with `HR_API_KEY` unset. Manually verified `ingestTrace` end-to-end against a real Postgres: a structured trace lands `compileStatus: READY` with steps persisted, in one request, no compiler configured. ## What is not yet covered `GET /api/recordings` gained a `take: 50` (P2-2 for this route) as an incidental fix while touching the file; the rest of that finding stands. Extension host permissions are `<all_urls>` for v1; scoping to allowlisted origins per organization is a natural follow-up once there is a customer to scope it for. No component tests for the extension itself — Manifest V3 content scripts have no test harness in this repo, so `roundtrip.test.ts` pinning its exact output shape is the coverage that exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(worker): make production container build and boot reliably The worker Dockerfile had never actually been built or run since it was written. Auditing what was genuinely left on the cloud roadmap surfaced two stacked bugs, neither caught by CI (`pnpm build` only bundles the worker with tsup at the workspace level — it never exercises the Dockerfile's own COPY list or executes the resulting dist/index.js): - The deps stage copied packages/core/package.json but not its prisma/ directory. @ghost/core's postinstall runs a bare `prisma generate`, which resolves the schema at the default ./prisma/schema.prisma path, so `pnpm install` failed inside the image before the build stage (which does copy the rest of packages/core) ever ran. The same gap silently dropped tsconfig.base.json, breaking apps/worker/tsconfig.json's `extends`. - Once building, the container crashed immediately on boot: bundling @ghost/core (via tsup's `noExternal: [/^@ghost\//]`) pulls in @prisma/client's generated CJS runtime, which dynamically requires native query-engine files — esbuild's CJS-to-ESM interop can't represent that and throws "Dynamic require of 'fs' is not supported" at the first call. Fixed by keeping @prisma/client external in tsup.config.ts and adding it as a direct dependency of @ghost/worker, since pnpm's strict linking won't resolve a transitive dep at the worker's own require path otherwise. Verified by rebuilding the image and running it against real Postgres/Redis until it logged its startup line rather than crashing. Added a CI step that builds and boot-smoke-tests the image on every PR so this class of bug can't ship invisibly again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(cloud): reconcile handoff and status documentation with current implementation CURSOR_HANDOFF.md described the WorkflowCompiler abstraction, four-eyes approval, and S3 artifact serving as open work, framing a stale roadmap that led to prioritizing already-shipped items. All three were fully implemented and tested by earlier PRs (#403, #406) whose changes never made it back into this file's "remaining work" section: - WorkflowCompiler: apps/web/src/lib/compiler/{types,index,harness-router-compiler}.ts already provides the interface, a swappable HarnessRouter adapter, and normalized error types. - Four-eyes approval: Membership/Role/Invitation plus Organization.requireSeparateApprover already enforce requester != approver server-side, covered by separation-of-duties.test.ts. - S3 artifact serving: packages/core/src/storage/artifacts.ts already has a working S3ArtifactStore with presigned URLs; only retention/cleanup is genuinely still open, called out as such. Also documents the worker container bugs found and fixed in the prior commit, and corrects the stale "239 tests" figure (actual full green run is 398) and the "typed step editor: remaining" status line (workflow-editor.tsx already covers it) in both this file and README.md. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(web): stop signin from silently rendering with no way to sign in In production with no GitHub OAuth app configured, the signin page rendered a card with a title and zero buttons -- indistinguishable from a bug. Whoever hits this is more likely to be standing up the deployment than an end user, so name the exact fix (AUTH_GITHUB_ID/AUTH_GITHUB_SECRET, the callback URL) instead of failing silently. See docs/DEPLOY.md's "sign-in trap". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(worker): add artifact retention, structured logging, and opt-in Sentry error tracking Three gaps identified by an audit of what was genuinely left on the cloud roadmap (recorded in CURSOR_HANDOFF.md), closed as one changeset since they share the same few files (index.ts, purgeArtifacts.ts): - Retention: nothing ever deleted a run's screenshots. `purge-artifacts` is a new BullMQ job, scheduled daily via upsertJobScheduler at worker boot, that deletes a run's artifact prefix once it ended more than ARTIFACT_RETENTION_DAYS ago (default 90) and audits the deletion. Run. artifactsPurgedAt makes it idempotent; a store failure is retried next cycle rather than silently marked done. Eligibility is a pure function (packages/core/src/retention.ts) so the window logic is tested without a database. - Structured logging: the worker only had console.log/console.error, so a failure was invisible unless someone was tailing container logs. packages/core/src/logger.ts is a small, dependency-free JSON-line logger (errors to stderr, everything else to stdout) now used throughout index.ts and purgeArtifacts.ts. - Error tracking: @ghost/core/sentry wraps @sentry/node, gated on SENTRY_DSN exactly like HR_API_KEY/S3_BUCKET -- absent means a complete no-op, present enables capture on every job-failure handler. Worker-only: wiring apps/web needs @sentry/nextjs (its own webpack/turbopack plugin exists specifically to handle Sentry's auto-instrumentation, which cannot otherwise be bundled -- confirmed by trying the manual approach first and watching it break `pnpm build` with an unbundleable node:child_process import). @sentry/node gets the same tsup `external` treatment as @prisma/client, for the same reason: its runtime does dynamic requires that bundling breaks. Verified end-to-end: full test suite (424 tests) against real Postgres + Redis, a Docker rebuild, and a boot smoke test with SENTRY_DSN actually set (not just absent) to confirm Sentry initializing doesn't crash the container. DEPLOY.md, README.md and CURSOR_HANDOFF.md updated for the new env vars and the corrected test count. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
mohabbis
added a commit
that referenced
this pull request
Aug 5, 2026
* feat(cloud): browser recording capture — Chrome extension + deterministic compile
Priority 3. Ghost could execute and approve, but nobody could record — the
only way to get a workflow into Ghost was to upload a trace file produced by
some other tool. There was no capture at all, and `RecordingStatus.ACTIVE` was
never set because nothing recorded.
Per the capture decision in docs/ARCHITECTURE_DECISIONS.md §3, this ships a
Chrome extension for v1, execution stays entirely server-side.
## The trace contract (`@ghost/core/recording/trace`)
A Zod schema for what a recorder uploads: navigate/click/input/select/submit
events, each carrying a `TraceTarget` with accessible role, name, test id and
ordered CSS fallbacks. The defining property: **role and name are read at
capture time**, off the live DOM while the element is still on screen — not
inferred later from an opaque trace. Those are exactly the fields
`resolveLocator` already prefers, so nothing downstream had to change.
A secret is never captured, not encrypted or truncated — absent. `redacted:
true` records that something was typed without recording what. This is the
only place that redaction can be done honestly: a trace carrying the value
plus a "please ignore this" flag has already leaked it (see the P0-2 finding
in the architecture doc, about the previous design uploading traces whole to
a third party).
## The compiler (`@ghost/core/recording/compile`)
Trace to typed steps, deterministically. No model, no network, no configured
compiler. Three things it does beyond translation:
- collapses a run of keystrokes on one field into one `fill` with the final
value
- runs every produced step through `classifyStep` — the same deterministic
classifier the worker consults at run time — and inserts an `approval`
immediately before anything it would gate, so the authored workflow
agrees with what execution will actually do
- never carries a redacted value; a secret field becomes a `fill` marked
`sensitive` with a placeholder, and a note tells the reviewer to set it
This is what makes recording work with no compiler configured — the state
production runs in per the HarnessRouter decision (§1). A model still has a
job: naming steps, proposing gates beyond the obvious cases, flagging what it
could not resolve. Deciding which element was clicked is not a judgement call
and should never have been one.
24 tests, including a fixture shaped exactly like the extension's actual
output (`roundtrip.test.ts`) — the seam most likely to rot, since the
extension is untyped JS with nothing else to notice if its output drifts from
what the compiler expects.
## Ingest (`lib/recording-ingest.ts`)
Shared by the existing upload form and the extension, so upload limits,
filename sanitisation and audit events cannot drift between the two paths. A
structured Ghost trace compiles inline and lands `READY` in the same request;
anything else (HAR, Playwright zip) is stored and left for whatever compiler
is configured, unchanged from before.
`POST /api/agent/recordings` is new: the extension's ingest point, on the
already bearer-authenticated agent surface rather than the session-only
upload route, since the extension runs at a different origin and cannot rely
on the session cookie reaching it. `resolveAgentPrincipal` enforces the same
second factor on its session fallback as every other agent route (see the
P0-1 fix). Uploading only creates a proposal — nothing here publishes a
workflow or executes anything; the human still reviews compiled steps in the
editor and publishes through `POST /api/workflows`, which revalidates them.
## The extension (`apps/extension`)
Manifest V3. `content.js` captures clicks, typing, selects, submits and SPA
navigation, computing accessible name in roughly accname-spec order from
label/aria-label/aria-labelledby/placeholder/value/text. Secret fields are
detected by `type=password`, secret-shaped `autocomplete`, or a name/label/id
matching a word list (password, otp, cvv, card number, ssn, ...) — and the
value is never read for them, not merely withheld after reading.
`background.js` buffers events locally and uploads only on Stop, via a
revocable bearer token created in Ghost Settings. The popup is deliberately
thin — start, stop, where to send it — because a second place to edit steps
would be a second thing to keep in sync with the real editor.
## Validation
398 tests pass (up from 374 — 24 new), against a database migrated from
zero. `pnpm typecheck` and `pnpm build` both clean with `HR_API_KEY` unset.
Manually verified `ingestTrace` end-to-end against a real Postgres: a
structured trace lands `compileStatus: READY` with steps persisted, in one
request, no compiler configured.
## What is not yet covered
`GET /api/recordings` gained a `take: 50` (P2-2 for this route) as an
incidental fix while touching the file; the rest of that finding stands.
Extension host permissions are `<all_urls>` for v1; scoping to allowlisted
origins per organization is a natural follow-up once there is a customer to
scope it for. No component tests for the extension itself — Manifest V3
content scripts have no test harness in this repo, so `roundtrip.test.ts`
pinning its exact output shape is the coverage that exists.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(worker): make production container build and boot reliably
The worker Dockerfile had never actually been built or run since it was
written. Auditing what was genuinely left on the cloud roadmap surfaced two
stacked bugs, neither caught by CI (`pnpm build` only bundles the worker with
tsup at the workspace level — it never exercises the Dockerfile's own COPY
list or executes the resulting dist/index.js):
- The deps stage copied packages/core/package.json but not its prisma/
directory. @ghost/core's postinstall runs a bare `prisma generate`, which
resolves the schema at the default ./prisma/schema.prisma path, so
`pnpm install` failed inside the image before the build stage (which does
copy the rest of packages/core) ever ran. The same gap silently dropped
tsconfig.base.json, breaking apps/worker/tsconfig.json's `extends`.
- Once building, the container crashed immediately on boot: bundling
@ghost/core (via tsup's `noExternal: [/^@ghost\//]`) pulls in
@prisma/client's generated CJS runtime, which dynamically requires native
query-engine files — esbuild's CJS-to-ESM interop can't represent that and
throws "Dynamic require of 'fs' is not supported" at the first call. Fixed
by keeping @prisma/client external in tsup.config.ts and adding it as a
direct dependency of @ghost/worker, since pnpm's strict linking won't
resolve a transitive dep at the worker's own require path otherwise.
Verified by rebuilding the image and running it against real Postgres/Redis
until it logged its startup line rather than crashing. Added a CI step that
builds and boot-smoke-tests the image on every PR so this class of bug can't
ship invisibly again.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* docs(cloud): reconcile handoff and status documentation with current implementation
CURSOR_HANDOFF.md described the WorkflowCompiler abstraction, four-eyes
approval, and S3 artifact serving as open work, framing a stale roadmap that
led to prioritizing already-shipped items. All three were fully implemented
and tested by earlier PRs (#403, #406) whose changes never made it back into
this file's "remaining work" section:
- WorkflowCompiler: apps/web/src/lib/compiler/{types,index,harness-router-compiler}.ts
already provides the interface, a swappable HarnessRouter adapter, and
normalized error types.
- Four-eyes approval: Membership/Role/Invitation plus
Organization.requireSeparateApprover already enforce requester != approver
server-side, covered by separation-of-duties.test.ts.
- S3 artifact serving: packages/core/src/storage/artifacts.ts already has a
working S3ArtifactStore with presigned URLs; only retention/cleanup is
genuinely still open, called out as such.
Also documents the worker container bugs found and fixed in the prior commit,
and corrects the stale "239 tests" figure (actual full green run is 398) and
the "typed step editor: remaining" status line (workflow-editor.tsx already
covers it) in both this file and README.md.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(web): stop signin from silently rendering with no way to sign in
In production with no GitHub OAuth app configured, the signin page rendered
a card with a title and zero buttons -- indistinguishable from a bug. Whoever
hits this is more likely to be standing up the deployment than an end user,
so name the exact fix (AUTH_GITHUB_ID/AUTH_GITHUB_SECRET, the callback URL)
instead of failing silently. See docs/DEPLOY.md's "sign-in trap".
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* feat(worker): add artifact retention, structured logging, and opt-in Sentry error tracking
Three gaps identified by an audit of what was genuinely left on the cloud
roadmap (recorded in CURSOR_HANDOFF.md), closed as one changeset since they
share the same few files (index.ts, purgeArtifacts.ts):
- Retention: nothing ever deleted a run's screenshots. `purge-artifacts` is a
new BullMQ job, scheduled daily via upsertJobScheduler at worker boot, that
deletes a run's artifact prefix once it ended more than
ARTIFACT_RETENTION_DAYS ago (default 90) and audits the deletion. Run.
artifactsPurgedAt makes it idempotent; a store failure is retried next cycle
rather than silently marked done. Eligibility is a pure function
(packages/core/src/retention.ts) so the window logic is tested without a
database.
- Structured logging: the worker only had console.log/console.error, so a
failure was invisible unless someone was tailing container logs.
packages/core/src/logger.ts is a small, dependency-free JSON-line logger
(errors to stderr, everything else to stdout) now used throughout
index.ts and purgeArtifacts.ts.
- Error tracking: @ghost/core/sentry wraps @sentry/node, gated on SENTRY_DSN
exactly like HR_API_KEY/S3_BUCKET -- absent means a complete no-op, present
enables capture on every job-failure handler. Worker-only: wiring apps/web
needs @sentry/nextjs (its own webpack/turbopack plugin exists specifically
to handle Sentry's auto-instrumentation, which cannot otherwise be bundled --
confirmed by trying the manual approach first and watching it break
`pnpm build` with an unbundleable node:child_process import). @sentry/node
gets the same tsup `external` treatment as @prisma/client, for the same
reason: its runtime does dynamic requires that bundling breaks.
Verified end-to-end: full test suite (424 tests) against real Postgres +
Redis, a Docker rebuild, and a boot smoke test with SENTRY_DSN actually set
(not just absent) to confirm Sentry initializing doesn't crash the container.
DEPLOY.md, README.md and CURSOR_HANDOFF.md updated for the new env vars and
the corrected test count.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(cloud): make cloud/apps/web actually deployable to Vercel
Discovered by performing the repo's first real deployment (Vercel project
ghost-app, Neon Postgres, Upstash Redis) rather than just writing a checklist
for one:
- cloud/apps/web needs its own vercel.json ({"framework": "nextjs"}).
DEPLOY.md previously claimed this wasn't necessary based on reading Vercel's
monorepo docs, but empirically: with the repo root's own vercel.json present
(the marketing site's static-build config) and Root Directory set but no
local vercel.json, a build falls back to the root's install/build commands
against the wrong project's uploaded files -- Vercel prints "The vercel.json
file should be inside of the provided root directory" as a warning, then
uses it anyway. A local vercel.json closes the gap outright.
- Added .vercelignore at the repo root. `vercel deploy` uploads the working
directory as-is, not `git ls-files`, so untracked local dev artifacts
(.worktrees/, .wt/, .turbo/ -- each a separate git-worktree checkout with
its own node_modules/target) get swept into the upload. Without this, a
deploy from the repo root uploaded 57,896 files instead of ~700.
- Vercel's own Deployment Protection (SSO/Vercel Authentication) was on by
default for the new project, gating every page behind a Vercel-account
login wall on top of Ghost's own auth -- disabled for ghost-app.
DEPLOY.md updated with all three findings, plus removing the stale "no
deployment exists yet" framing now that apps/web is live (worker is not; no
container host is wired up, and object storage was deliberately deferred).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* ci: stop requiring the marketing site to advertise a desktop release
The version-consistency check hard-failed when public/index.html didn't
contain a vX.Y.Z string, cross-checked against README.md. That assumption
broke on master after the site copy was rewritten to drop legacy desktop
positioning entirely (following the download-CTA removal in #396) -- the
site no longer mentions a specific release at all, which is the intended
current state, not drift.
README.md still names the published desktop tag as historical reference;
nothing there needs to change. Only cross-check the two when the site
actually advertises a version, so a real future mismatch still fails loud.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
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
1 participant
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.
Two related pieces of work: the Convert half of Phase 2 recording, and the auth hardening that surfaced while testing it.
1. Recording → editable workflow steps (
feat)Builds the Convert half of Phase 2 (
docs/CURSOR_HANDOFF.md): upload the raw trace of one demonstrated workflow, have a purpose-configured HarnessRouter agent propose a typed step plan, review and edit it in the existingWorkflowEditor, then publish through the existingPOST /api/workflowspath.The proposal is a proposal. It is validated against
@ghost/core/schema/stepbefore it is stored, it is never executed, and publishing re-validates it exactly as it would a hand-authored definition. The AI pre-fills the editor and nothing more — the trust pipeline is unchanged.Recordinggains a compile lifecycle (RecordingCompileStatus) separate from its capture status, so aSTOPPEDrecording can be recompiled after a failure without pretending the capture needs to happen again.Capture remains the open Phase 2 decision (remote browser vs. extension). This sits behind whatever produces the trace file.
Notes on the HarnessRouter integration
HR_API_KEYis read server-side only. File bytes are relayed through our own authenticated routes rather than handing the browser an API-key-protecteddownload_url.POST /v1/filesreturns the upload id asid, notfile_id— the session files API usesfile_idfor a different id entirely. Reading the wrong key does not throw: it yieldsundefined, theinput_fileblock is still accepted, and the agent runs with no attachment, reporting only that it cannot find the file.uploadFilenow fails loudly and a test pins it.data.type(there are noevent:lines) and ignores unknown types.startResponsereturns once it has the recovery identifiers rather than holding the connection open; closing it does not stop the run.RUNNINGforever.Verified end to end through the running app
Upload → compile → streamed progress → review → publish. The compiler emitted semantic
role+nameselectors (no coordinates) and inserted an approval gate before a "Pay now" click:{ "id": "step-6", "type": "approval", "reason": "About to submit payment ('Pay now'). This is an irreversible financial transaction." }Tenant isolation: cross-tenant access to every recording route returns 404, the stream reports
notFound, and claiming another org's recording while publishing is refused with 409 (nothing created). Org audit chain verifies intact across the run:recording.uploaded → compile_started → compile_failed → compile_started → compile_ready → workflow.version_published.2. Fail closed on unsafe auth configuration (
fix)Three things that failed silently — app boots, sign-in works, nothing looks wrong.
A published session secret.
.env.exampleships a workingAUTH_SECRET(deliberate, same trade-off asGHOST_SESSION_KEY). It is in this repository, so anyone holding it can forge a session for any user in any organization. Production now refuses to start on it, on an unset value, or on anything under 32 characters. Outside production it only warns — breakingpnpm devover a dev secret would just push people back to the published one.next buildis exempt: it runs withNODE_ENV=productionand imports every route module to collect page data, so asserting there makes a real secret a prerequisite for compiling, including in CI. Building is not serving; the check still runs on first import to handle a request.A dev provider gated on one variable. The passwordless "any email" provider was registered whenever
NODE_ENV !== "production"— one variable, routinely unset, that nobody treats as a security boundary. It now also requires the instance to be loopback-only (viaAUTH_URL) and honours an explicit off switch, so forgetting any single signal fails safe.A matcher that silently drifts.
/auditand/recordingswere both missing from the middleware matcher. Nothing leaked —(app)/layout.tsxredirects when there is no session, and that is what actually stops an unauthenticated render — but two guards disagreeing stays invisible until the day the layout stops being the thing that saves you. Both added, andmiddleware.test.tsderives the expected list from the filesystem so a new page without an entry fails the suite (verified by removing an entry and watching it fail).Session lifetime drops from Auth.js's 30-day default to 12 hours.
This is the first of four agreed auth workstreams; invites/RBAC, MFA/SSO, and per-tenant identity to the runtime agent are still to come.
Validation
pnpm typecheck,pnpm test(297 tests),pnpm build— all pass. Local dev sign-in re-verified after the auth change.Tests ran against a clean, freshly migrated Postgres. Note for reviewers: the shared local
ghost-postgresdev container has drifted frommaster's migration history (missingOrganization.auditChainHead, plus anexecution_plan_v1migration not in this repo), which fails many pre-existing tests regardless of this branch. Unrelated to these changes and not touched here.🤖 Generated with Claude Code