Repository navigation
fix: make invite acceptance work for new invitees and workspace roles manageable - #578
Conversation
…kspace roles manageable Invitation acceptance was impossible for anyone who was not already signed in (#534). The acceptance page dropped the ?token query when it bounced an unauthenticated visitor to sign-in, and the shared next-target allow-list only accepted the bare /invitations/accept path with no query arm, so the token never came back. The page then told the invitee to request a fresh link, which failed in exactly the same way. The token now travels in the next value across both the sign-in and the sign-up round trip. The allow-list gains /invitations/accept as a prefix entry beside /oauth/consent, which matches only at a real path or query boundary, so off-site and lookalike targets are still refused. Acceptance failures now carry a stable machine code from the control-plane (expired, already accepted, email mismatch, not found) and the page states the real reason with an action that can actually resolve it, instead of always asking for a new link. The members page never read its own searchParams, so a granted or failed invite was silent (#535). It now renders a confirmation for a sent invite, a joined workspace and an updated role, and renders a failure as an alert. Roles were unmanageable from the console (#536). The invite form gains a role selector, existing members can be re-roled, and the Member column shows the member email instead of a raw UUID. Every role change is authorized server-side through the existing authz policy (members.manage), and two invariants are enforced in the service and again in SQL: nobody may change their own role, and the last active owner can never be demoted. Disabled controls now state their real reason rather than blaming email verification for a permission gate. Closes #534 Closes #535 Closes #536
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Warning Review limit reached
Next review available in: 12 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (15)
📒 Files selected for processing (36)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…d stop the members page crashing for them Accepting an invitation for a workspace the user already belongs to hit the membership unique constraint and surfaced as an opaque server error. The condition is knowable, so it is now detected before the write and reported as invitation_already_member with copy that points at the workspace switcher. A plain member visiting the members page also hit the console error boundary, because the control-plane restricts the member list to owners and the client threw. The refusal is now an explanation on the page, while any other failure still throws so a real outage stays visible. Adds the invite-journey visual proof captured against a running stack.
PR #578 committed four real invitation tokens as `?token=` query params in a captured window.location.href overlay, inside docs/proof/invite-journey-534. GitGuardian caught it after the fact. Add tools/lint-no-token-in-proof-captures.mjs, wired into the repo-policy-lints CI job as lint:proof-tokens. It scans tracked text files under docs/proof/ for token=, access_token=, refresh_token=, and code= followed by a long, high-entropy value, and fails the build if one looks real. It cannot inspect screenshot pixels, so also note in the visual-proof-before-merge rule that any credential-bearing URL must be redacted in both the text log and the screenshot overlay before commit. Self-test (MUST_CATCH / MUST_ALLOW fixtures) runs as a preflight on every invocation, mirroring tools/lint-no-request-url-origin.mjs.
63df639 to
175fa58
Compare
…-and-member-roles # Conflicts: # .github/workflows/ci.yml
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
…the right image Three review findings. The captures artifact uploaded on always(), so a credential the lint had just caught was blocked from the pull request comment and published to a public artifact anyway, which is the PR #578 leak class with an extra step. The upload is now gated on the lint, which itself fails the job. The SIF came from whatever agent-engine-sif artifact was newest repo wide, with no relation to the code under test, so a pull request that rebuilds the image would have been certified with a stale one. The artifact is now selected by image inputs, hashing the deploy/apptainer, packs and vendor/openhands trees at this ref against the trees at the artifact's own commit, and a run with no matching artifact fails and says how to build one. redact-log-credentials.py had no pattern for a bare key, so S3_ACCESS_KEY, S3_SECRET_KEY, LITELLM_MASTER_KEY and SUPABASE_ANON_KEY all passed through into the public failure-log artifact. Added, with the five spellings this job wires in covered by the self-check.
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
Image assets only. No application code changes, no test changes, no configuration changes. ## What this is Eleven captures collected on 2026-08-17, paired so that the UX issues filed the same day can show the gap rather than describe it. Four are Claude.ai, which the owner named as the target experience, and seven are `chat-hive.scubed.co` as it is deployed today. The specification these support is `spec-2026-08-17-target-ux-claude-desktop-parity` in the project vault, which is authoritative on what we build. This directory is only the evidence. ## Why it lands as its own pull request The issues embed these images by raw URL. That means the blobs have to exist on the remote before the issues can be written, and putting them in whichever feature branch happens to fix one of the defects would tie eight issues to one branch's lifetime. ## Redaction This repository is public, and PR #578 already leaked live credentials through committed proof artifacts once, so the inspection was treated as the load-bearing step rather than a formality. Every image was opened and looked at before it was staged. Three crops were applied, and each one removes the owner's personal data rather than obscuring it in place: - `target-chat-home.png` and `target-model-picker.png` are cropped to the greeting and the composer, so the organization identifier rendered above them is not reproduced. - `target-sidebar-nav.png` is cropped above the Pinned heading, so a pinned private project name is not reproduced. One target capture is omitted entirely. It showed the settings General page, and it carried the owner's custom instructions text in full plus a column of private conversation titles down the left edge. No crop of it was worth publishing, and nothing in the issues needs it: the one fact it settles, that the target has a "What should Claude call you?" field, is stated in the specification and cited from there. A further twenty seven captures of the target exist on the owner's machine and are deliberately not in this diff. Most carry a private job search and client data. They are described in the specification and are not republished. `npm run lint:proof-tokens` passes, which is the automated half of this check. It reads text and cannot inspect pixels, which is why the manual pass above happened first. ## Verification - `npm run lint:proof-tokens` — ok, 77 files under `docs/proof/` scanned. - Every committed image opened and inspected after cropping, not only before.
…#973) deckgen.Render and artifactsclient.Client both existed, both tested, and had zero non-test callers anywhere in the repo. An agent could produce a deck and nothing stored it, nothing could address it afterwards, and the user could not open it: `agent_tasks.result_summary_ref` was always the agent's raw final-response text, and the task console printed it as if it were a link (`task-console.tsx:621`). This wires the two subsystems together in the process that actually executes on the demo box: the socket-arm host launcher (`apps/agent-engine/cmd/agent-engine -serve`), not the in-process arm, which cannot run under docker compose regardless of configuration. - `SandboxEngine.Status` (`apps/agent-engine/internal/engine/engine.go`), on a knowledge-work-pack task's terminal success and before its `/workspace` bind mount is reaped, looks for exactly one declared output file: `.hive/deck.json`. Never a directory scan, never a glob. If present and valid, it is rendered with the already-tested `deckgen.Render` and published via `artifactsclient.Client.Create` (or `AddVersion` when the manifest names an existing artifact id), authenticated as the task's own user. - That authentication needed a bearer JWT that never reached this process before. It is captured once, in `apps/edge-api/internal/agenttask/handler.go`'s `handleCreate` (the only place that ever sees the raw `Authorization` header), and threaded in memory only through `control-plane`'s `CreateTask` and the agent-engine daemon's `/launch` call. It is never persisted to `public.agent_tasks`, never exposed to the sandboxed agent, and an API-key-authenticated create (no Supabase JWT to forward) just skips publishing rather than forwarding an API key as if it were one. - Ownership is enforced entirely by code this PR does not touch: edge-api's `/v1/artifacts` resolves `tenant_id` strictly from that same JWT's claims, and `AddVersion`'s tenant-scoped RLS lookup 404s outright on a manifest-supplied `artifact_id` from another tenant. No new internal-token bypass was introduced anywhere. - A publish failure of any kind (no manifest, malformed or oversized manifest, edge-api unreachable or rejecting it) falls back to the agent's own final-response text and never turns a succeeded task into a failed one. A manifest over 1 MiB is rejected outright, never silently truncated. - `apps/agent-console`'s task console renders `result_summary_ref` as a real `<a>` when it is an artifact-shaped path. The href points at this app's own deck proxy rather than the artifacts origin, because a deck is a private artifact and a browser navigation carries no `Authorization` header; see the update below for why the original artifacts-origin link could not open. - `scripts/install-agent-engine-host.sh` and `deploy/docker/docker-compose.yml` wire the one new environment variable this needs (`EDGE_API_URL`, host-reachable, mirroring how `CONTROL_PLANE_URL` is already overridden there). Left unset, the whole feature is a no-op with zero behavior change (see the review-fix update below for why an earlier version of this claim was briefly false). - `apps/agent-engine/packs/knowledge-work-pack/skills/deck-generation/AGENTS.md` is corrected to describe what is actually true: the sandbox has no Go toolchain and no network route to edge-api, so the agent's real job is writing the manifest, not calling `deckgen.Render`/`artifactsclient.Client` itself. ## Update: review findings fixed (a39d72c) Two Go-review bugs and five security-review findings came back. All fixed, all replied to inline. Summary: - **Kill switch (Go review, was a real bug):** `EDGE_API_URL` previously could not be turned off (`envOr`'s fallback masked "explicitly empty" as "use the default", and the default was a live, correct address under the demo box's deployment path). The claim above that this feature is opt-in and off-by-default was false at the time it was first written. `serve.go` now reads a plain `os.Getenv` with no fallback: unset means off, full stop. The install script's own default now uses `${VAR-x}` so an explicit empty override is honored. - **Idempotent publish (Go review):** two concurrent `Status` calls for the same session could each independently publish an artifact for the same task, orphaning one of them. Fixed with a per-session mutex serializing the terminal-transition work, never the engine-wide lock. New test forces the interleaving deterministically. - **TOCTOU (security review, HIGH, blocked merge):** the prior symlink-escape fix (`resolveWithinRoot`) checked a path string and reopened that same string fresh, an unsynchronized two-step sequence a live sandboxed process could race. Replaced with Go 1.24's `os.Root`, which resolves and opens in one syscall. New test hammers a real rename/symlink swap for 500 iterations. - **FIFO hang (security review, MEDIUM-HIGH, blocked merge):** `mkfifo .hive/deck.json` would hang the open forever with no race required, leaking the goroutine, the quota slot, and the whole sandbox container. Fixed with `O_NONBLOCK` plus a regular-file check on the opened descriptor. - **Credential lifetime (security review, MEDIUM):** the bearer JWT was never cleared from the long-retained session struct. `reap()` now zeroes it on any terminal transition. - **Silent expiry (security review, LOW):** added a distinguishable log line (and an `ErrUnauthorized` sentinel in `artifactsclient`) for a 401/403 publish rejection, so an expired-token failure mode doesn't read the same as any other transient error. ## Update: the published link did not open, and now does (576292e onward) The rebase pass turned up two defects and one design gap. The gap is the important one: **as previously written, this pull request's headline outcome did not work.** ### The gap: a private artifact behind a header-only route A published deck is private (`artifacts.is_public` is `NOT NULL DEFAULT false`) and edge-api's serving route resolves a viewer only from an `Authorization: Bearer` header (`optionalViewerTenant`). There is no cookie path anywhere in `internal/artifacts` or `internal/auth`, and `Caddyfile.artifacts` is a bare reverse proxy that injects no credential. A browser navigation carries no such header, so the owner following their own deck link was an anonymous viewer and RLS answered 404. This was proven rather than argued. `TestRouteServe_PrivateArtifact_NoAuth404` passes unmodified; flipping only the artifact's privacy, and nothing else, turns it red with `status = 200, want 404`; restoring returns it to green. That 200 under mutation is the same request the anchor issued. It also mattered more than a missing link, because this change **replaces** `result_summary_ref` with an opaque `/artifacts/{id}` path where it previously held the agent's readable final-response text. Merging without a working link would have been a regression against main, not a partial landing. Two shortcuts were considered and rejected on the record: - **Publishing decks anonymously readable.** Rejected. It makes an unguessable UUID the access control, which is the pattern Hive Enterprise's data-sovereignty posture exists to reject, and it would contradict #960 tightening cross-tenant reads in the same week. It also runs against a recorded decision: `.wolf/decisions.md` D-015 specifies the public artifact link tier as **owner-toggled visibility**, so flipping a deck public automatically is precisely the thing that decision reserves for an explicit owner action. - **A short-lived signed URL.** Priced, then rejected. No signing primitive exists to reuse: the only `crypto/hmac` in the tree is `signWebhook` (an outbound webhook body digest, no expiry, no resource scoping), and `S3Client.PresignedURL` is SigV4 against the S3 *object* URL, so it would serve tenant HTML from a shared storage origin carrying none of `writeArtifactHeaders`'s CSP. That is a worse posture than the option already rejected. Building it properly means a new secret, a mint endpoint, a verification branch and a credential permanently living in a query string, the shape that leaked four real invitation tokens on PR #578. ### The fix: a same-origin deck proxy `GET /agent-workspace/api/deck/{id}[/v/{n}]` (`apps/agent-console/app/api/deck/[...ref]/route.ts`) resolves the caller's existing Supabase session server side and re-attaches that same access token on a server-to-server call to edge-api. Nothing is minted and nothing is loosened: the viewer must still be signed in as the owning tenant, and edge-api still decides. Decks stay private. Same-origin is load bearing, not incidental: `/artifacts/*` is not on this app's Caddy route and edge-api emits no CORS headers, so a browser-side fetch could not reach it at all. The response mirrors `writeArtifactHeaders` and adds `sandbox allow-scripts`. That is the security-critical line. Serving tenant-authored HTML from the app's own host would otherwise place it in the app's origin with reach into its cookies; `allow-scripts` with no `allow-same-origin` drops it into an opaque origin, so the deck's inline navigation JS still runs but can touch nothing of ours. It restores exactly the isolation the separate artifacts origin was providing. `parseArtifactRef` is now the single place a ref is judged artifact-shaped, shared by the link builder and the route, so the two cannot drift into trusting different shapes. `BASE_PATH` is applied explicitly because Next does not prepend `basePath` to raw hrefs, and without it the link would fall through to Open WebUI's catch-all. No Caddy or infrastructure change is required: `@agentConsole path /agent-workspace /agent-workspace/*` already routes it, and `EDGE_API_INTERNAL_BASE_URL` is a compose `environment:` entry with a literal `http://edge-api:8080` default, so a deploy picks it up with no operator `.env` edit. `NEXT_PUBLIC_ARTIFACTS_BASE_URL`, added earlier in this pull request, now has no readers and is removed along with its Dockerfile build arg. ### Two defects fixed on the way - **Data race on the task bearer JWT.** `publishDeckArtifact` read `sess.bearerJWT` without `e.mu` while `reap` clears it under `e.mu`, and `Cancel` reaches `reap` without ever taking `sess.finishMu`, so a cancel landing as a task finished raced the publish. The read now goes through an `e.mu`-guarded accessor. Confirmed by mutation: the unsynchronized read produces `WARNING: DATA RACE` naming `engine.go:824` against `engine.go:687`; the fix is green across twelve packages under `-race`, which CI runs. - **Local artifact origin.** The fallback pointed at port 80 while `caddy-artifacts` publishes on 3004. Moot now that the variable is gone, but it was real. ### The middleware nearly deleted the fix's one security control Recorded at length in the buglog entries below, and worth stating here because the next person adding a route will meet the same edge. `apps/agent-console/middleware.ts` runs on every path except static assets and calls `res.headers.set("Content-Security-Policy", "frame-ancestors 'self'")`. **`set` replaces, it does not merge.** So the middleware overwrote the deck route's own policy wholesale, deleting `sandbox allow-scripts`, which is the single directive keeping tenant-authored HTML out of this app's origin. Without it the deck would have executed inline script in the console's origin with reach into its cookies and storage: a stored-XSS-shaped hole on the surface that holds admin sessions. Nothing failed and nothing logged. The route's unit tests could not see it. They exercise the exported `GET` handler and observe the `Response` it returns; middleware runs outside that boundary and mutates the response afterwards. Every assertion about the route's CSP passed while the header actually served to a browser was a different, weaker one. It was caught by building the production image, running `next start` and curling it. **This is header precedence, and it is reintroducible.** Any future route handler that writes its own `Content-Security-Policy` will have it silently replaced unless its path is exempted. The fix is an explicit path allowlist (`/api/deck/`) rather than a "skip if the response already has a CSP" check, and that choice is deliberate: the allowlist is fail-safe, hardened by default with an explicit opt-out, whereas a presence check is fail-open, letting any future handler that sets any policy silently opt itself out of clickjacking protection too. The trade is that a new route must remember to exempt itself; the middleware comment now says so at the point of the decision. One broader note, since it bears on how this feature was reviewed. A was rejected for widening access through the URL, and B then nearly widened access through the origin instead. This class of feature, hosting untrusted tenant-authored HTML, leaks by default, and every path to it needs the same scrutiny rather than only the one that looked risky first. One note on the regression guard, because it nearly shipped unable to fail: its first version waited on the publisher's `started` channel before cancelling, which ordered the read before the write and removed the race it claimed to test. It passed against deliberately broken code. The committed version parks the engine inside the final-response fetch, the last step before the read, and cancels from there. ## Test plan - [x] `go build`/`go vet` clean across `apps/agent-engine`, `apps/control-plane`, `apps/edge-api`. - [x] `go test` green: `apps/agent-engine/internal/engine` (26 tests now, including the concurrency, TOCTOU-swap, and FIFO regression tests added in the fix round), `.../deckgen`, `.../artifactsclient`, `apps/control-plane/internal/agenttask`, `.../agentengine`, `apps/edge-api/internal/agenttask`. - [x] `apps/agent-console` vitest suite green (75 passed). `npx tsc --noEmit` clean. - [x] `go test -short -race` green across the agent-engine, control-plane agenttask and edge-api agenttask packages (twelve packages, no warnings), including the new `TestSandboxEngine_Status_CancelDuringPublishIsRaceFree`. - [x] `apps/agent-console` type check, unit suite and production build green, including the new deck-proxy route tests. - [ ] **Live visual proof outstanding, and this pull request is held for it.** The capture must show a Cowork task producing a deck AND the link opening, which needs a real Apptainer sandbox. The only pre-merge mechanism for that is `.github/workflows/agent-visual-proof.yml`, which currently fails for every pull request, including unrelated dependabot branches, at `Start the stack` with control-plane reporting `database not available at startup: database unreachable after 13 attempt(s) over 1m15s`. That is environmental and tracked separately as issue #1050; this branch is not the cause. A capture showing only the link opening was deliberately not substituted: it would prove the smaller half and still leave the real one owed. ## Buglog entry ```json {"date":"2026-08-18","error_message":"agent_tasks.result_summary_ref always held the agent's raw final-response text; a knowledge-work-pack deck was rendered nowhere and linked nowhere","root_cause":"deckgen.Render and artifactsclient.Client (Create/AddVersion) had zero non-test callers; no bearer JWT ever reached the agent-engine host process to authenticate a publish call, and no file-output convention existed between the sandboxed agent and the host process","fix":"added Task.BearerJWT threaded from edge-api's task-create handler through control-plane to the agent-engine daemon's /launch call; SandboxEngine.Status reads a single well-known .hive/deck.json manifest before reaping /workspace, renders it, and publishes via artifactsclient, overriding resultSummary with the artifact URL only on success; task-console now linkifies artifact-shaped refs via NEXT_PUBLIC_ARTIFACTS_BASE_URL; review fixes added a real off switch, idempotent publish via a per-session lock, os.Root-based TOCTOU-proof manifest reads, FIFO rejection, bearer-JWT lifetime bounding, and a distinguishable log line for expired-token publish failures","tags":["agent-engine","artifacts","cowork","knowledge-work-pack","result_summary_ref"]} ``` Additional entries from the rebase and deck-proxy pass: ```json {"date":"2026-08-23","error_message":"data race between Cancel and a knowledge-work-pack publish on the agent-engine session bearer JWT","root_cause":"publishDeckArtifact read sess.bearerJWT without holding e.mu while reap clears that same field under e.mu; Cancel reaches reap without ever taking sess.finishMu, so a user cancelling a task at the moment it finishes puts the write concurrent with the read with no happens-before edge between them, which the race detector fails the build on and which could hand Create a half-cleared credential","fix":"read the credential once through a new bearerJWTOf accessor that takes e.mu and use that copy for the rest of publishDeckArtifact; regression guard TestSandboxEngine_Status_CancelDuringPublishIsRaceFree holds the engine inside the final-response fetch, the last step before the read, and launches Cancel from there","tags":["agent-engine","concurrency","data-race","cowork","artifacts"]} ``` ```json {"date":"2026-08-23","error_message":"a race regression test passed against deliberately unsynchronized code","root_cause":"the first version of TestSandboxEngine_Status_CancelDuringPublishIsRaceFree waited on the publisher's started channel before launching Cancel; Create runs after the bearer JWT is read, so that channel send established a happens-before edge that ordered the read before the write and removed the very race the test claimed to cover","fix":"gate on the fake agent server's final-response handler instead, which parks the engine immediately before the read, so the cancel is genuinely concurrent with it; verified by mutation, the unsynchronized read now produces WARNING: DATA RACE naming engine.go:824 against engine.go:687","tags":["testing","false-green","race-detector","guard-cannot-fail"]} ``` ```json {"date":"2026-08-23","error_message":"published deck artifact link returns 404 for the user who owns it","root_cause":"publishDeckArtifact calls Create and never marks the artifact public, artifacts.is_public defaults to false, and the serving route resolves a viewer only from an Authorization Bearer header (optionalViewerTenant) with no cookie path anywhere in internal/artifacts or internal/auth; a browser navigation sends no such header, so the viewer is anonymous and RLS admits only public rows","fix":"serve the deck through a same-origin server-side proxy in agent-console that resolves the caller's Supabase session server side and calls edge-api with that user's own bearer, returning the HTML under Content-Security-Policy sandbox allow-scripts for an opaque origin; rejected making decks public (data sovereignty) and a signed URL (no signing primitive, credential in query string)","tags":["artifacts","auth","cowork","rls","csp"]} ``` ```json {"date":"2026-08-23","error_message":"NEXT_PUBLIC_ARTIFACTS_BASE_URL fallback unreachable in local and enterprise profiles","root_cause":"the fallback resolved to http://artifacts.localhost, port 80, while caddy-artifacts publishes on host port 3004, so the browser was handed a link that could not connect before authentication was even reached","fix":"carry the published port in the fallback, http://artifacts.localhost:3004, leaving the configured-ARTIFACTS_DOMAIN path unchanged","tags":["docker-compose","artifacts","config"]} ``` ```json {"date":"2026-08-23","error_message":"agent-console middleware silently replaced the deck proxy's Content-Security-Policy, deleting the sandbox directive that keeps tenant HTML out of the app origin","root_cause":"middleware.ts runs on every path except static assets and calls res.headers.set(\"Content-Security-Policy\", \"frame-ancestors 'self'\"); set REPLACES rather than merges, so a route handler that writes its own policy has it overwritten wholesale after the fact. The deck proxy serves tenant-authored HTML from the console's own host and depends on sandbox allow-scripts for an opaque origin; with that directive stripped the deck would have executed inline script in the console's origin with access to its cookies and storage, a stored-XSS-shaped hole on the surface that holds admin sessions. Nothing failed and nothing logged","fix":"exempt /api/deck/ from the middleware's header stamping so the route's own strictly narrower policy survives; the route sets frame-ancestors 'none', so no protection is lost. Kept as an explicit path allowlist rather than a has-CSP presence check on purpose: the allowlist is fail-safe (hardened by default, opt out explicitly) while a presence check is fail-open (any future handler that sets any CSP silently opts itself out of clickjacking protection)","tags":["security","csp","xss","middleware","header-precedence","agent-console","false-green"]} ``` ```json {"date":"2026-08-23","error_message":"route unit tests could not detect a security header being stripped by middleware","root_cause":"vitest exercises the exported GET handler directly, so it observes the handler's own Response object; Next middleware runs outside that boundary and mutates the response afterwards, so every assertion about the route's Content-Security-Policy passed while the header actually served to a browser was a different, weaker one","fix":"verify security headers against a running production image (docker build, next start, curl) rather than against the handler in isolation; unit assertions on response headers are necessary but prove nothing about what the server emits once middleware composes the response","tags":["testing","false-green","middleware","integration-gap","security"]} ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Generated knowledge-work decks are now published as accessible artifacts. - Task results display valid deck artifacts as secure links that open in a new tab. - Supports creating new artifact versions and viewing versioned decks. - Added an authenticated same-origin deck viewer with access validation and security protections. - **Bug Fixes** - Invalid, missing, oversized, or unavailable deck manifests no longer replace successful task results with broken links. - Improved handling of concurrent task completion and artifact publishing failures. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
… guarding Two false failures in the first CI run that measured anything, both caused by the guard rather than by the console. An interceptor registered for the whole run sits in front of every request even when it lets one through, and this suite's job is to observe what a page does on its own. Two sidebar and card links on /console reported no request and no navigation in CI while the same links proved by an RSC fetch against the live console. The guard is now registered for the activation it guards and removed straight after. A blocked write reveals the application's network-error branch, and its Try again button exists only while that error is on screen. Three of those were queued and then reported as unlocatable on a fresh page, which is the gate accusing the console of a control the gate itself conjured. Reveals are not followed when the guard fired. Also redacts credential-shaped query parameters and drops the fragment from every URL the ledger records. The ledger is committed and uploaded from a public repository, and PR #578 published four live invitation tokens by writing a URL down verbatim.
CodeQL flagged js/incomplete-sanitization (high) on the report writer: it escaped a pipe by prefixing a backslash but never escaped the backslash itself, so `\|` in an observation string came out as a literal backslash followed by a live cell separator. A newline does the same damage one row up, since GFM ends a table row at the newline and a multi-line error message shreds the rest of the table. Both are now handled by mdTableCell, a shared helper that escapes in a single pass, flattens newlines, and refuses to leave an orphan backslash behind when it truncates. Every cell goes through it, not just the observation column, and the helper has its own unit test. The adversarial pass over the rest of the diff found a latent credential leak of the PR #578 class. Step 11 mints a real API key, and the console renders a newly created key in full exactly once, in the element the harness then photographs as 11c-console-api-key-created.png and commits under docs/proof/. That never fired only because the create call failed on every run so far. The step now reads the secret from its own element rather than regexing the page text, masks that element in the DOM before the shutter, registers the key as a run secret so it is scrubbed from every log and report cell, and sets the form's own expiry field so a key that outlives its run expires within a day. Three smaller findings from the same pass: - The owner-signup step defaulted to a hardcoded address and set a password on whatever it resolved to. That is the hazard E2E_RUN_KEY exists to prevent, so HIVE_OWNER_SIGNUP_EMAIL is now required with no default, and the password it sets is registered as a run secret. - The attachment fixture was written into the committed proof directory rather than a temp dir. - The minted key was handed between steps through a global. Finally, the proof-capture linter could not see a bare hk_ key at all: it scans query parameters and JWTs, and an API key rides in an Authorization header. It now catches one, with self-test fixtures for both the live shape and the masked and redacted shapes that must stay quiet.
…1417) Fixes #1409. ## What was wrong `repo-policy-lints` is a required check declared with `if: always()`, and every step in it carried `if: needs.changes.outputs.run != 'false'`. On a docs-only pull request the job therefore reported success having executed nothing. One step in that job guards a docs path. `tools/lint-no-token-in-proof-captures.mjs` resolves its scan root to `docs/proof/` and reads nothing else, and `docs/*` is on the inert-path allow-list that computes the docs-only verdict. So the only automated guard standing in front of a credential committed into a proof capture switched itself off on the exact pull-request shape that adds proof captures. PR #1398 committed seventy-one captures plus a README. Its `Repo policy lints (tenant + audit)` check reported pass in six seconds, and it was merged on that green. Running the linter by hand on merged `main` afterwards reported `ok (217 files under docs/proof/ scanned)`, so nothing had leaked. Safe by luck, not by gate. ## Answers to the three questions on the issue ### 1. Where the verdict is computed, and what it short circuits `.github/workflows/ci.yml`, the `changes` job, step `Decide whether the real suite must run`. It lists the pull request's files through the GitHub API and classifies each one. An executable extension (`*.js`, `*.mjs`, `*.cjs`, `*.jsx`, `*.ts`, `*.mts`, `*.cts`, `*.tsx`, `*.sh`, `*.py`) is denied first and forces `run=true`. Then `*.md`, `LICENSE`, `NOTICE` and anything under `docs/`, `.wolf/`, `.claude/`, `.vscode/`, `.cursor/` are inert. Anything unmatched forces `run=true`. Jobs skipped wholesale when the verdict is docs-only (`if: needs.changes.outputs.run == 'true'`): `rust-tests`, `desktop-tests`, `agent-console-unit`, `docker-build`, `markitdown-sidecar`, `live-integration`, `web-e2e`, `interaction-coverage`, `egress-firewall-e2e`. Every one of those builds or exercises code, so none of them has the "the thing I guard is a docs path" character. Jobs always scheduled but gated per step: `go-tests`, `web-unit`, `repo-policy-lints`. I checked the rest of `repo-policy-lints` for the same hole, since the concern is not this one linter. Its other guards read workflows, Dockerfiles, shell scripts, Go sources, `package.json`, or `.claude/hooks/*.js`. Every one of those inputs already forces `run=true`, either through the extension-deny arm (`.js`, `.mjs`, `.sh`, `.py`) or through the unmatched-path arm (`.github/workflows/*.yml`, `deploy/docker/Dockerfile.*`, `Makefile`). `lint:proof-tokens` is the single guard in the tree whose subject lives under an allow-listed prefix, so it is the only one with this defect today. Nothing else needs naming, and nothing else was left unfixed. ### 2. Which fix, and why Three candidates: 1. **Exempt `docs/proof/**` from the docs-only classification.** Rejected. It would run the entire heavy suite, `docker-build` and `web-e2e` included, over a pull request of screenshots, to gain one sub-second filesystem scan. It also pays that price for the image half of a capture, which the scanner cannot read at all. 2. **Run `lint:proof-tokens` unconditionally.** Chosen. 3. **Add a third `changes` output that is true when a `docs/proof/**` path changed, and gate the step on it.** Rejected. It copies the scanner's scope into the workflow, and the allow-list comment a few lines above already states the governing rule, that a second copy of a list is a second thing to forget. It is also strictly weaker than option 2: the scanner walks the committed tree rather than the diff, so an unconditional run also covers a capture that landed earlier and was never scanned, which a diff-scoped gate cannot. Cheapest measured against the failure mode, which is the instruction on the issue. The failure mode removed is a credential published to a public repository with no other automated backstop at all, since GitHub secret scanning, push protection and GitGuardian do not inspect a release asset and inspect no pixels anywhere. PR #578 is the precedent, four live invitation tokens, and that pull request was very likely docs-only itself. Every other step in the job keeps its gate, so a docs-only pull request still skips the tenancy, audit, deploy and workflow guards that genuinely have nothing to do with it. ### 3. The other direction Yes, and this pull request does not close it. The scanner reads `docs/proof/` by design and by stated rule, so a capture log committed anywhere else is unscanned no matter which checks run. This fix should not be read as more complete than it is: it fixes when the scanner runs, not where it looks. Filed separately as #1420, with the candidate designs and the same red-first acceptance bar. ## What this costs **On an ordinary code pull request, nothing.** `run=true` there, so checkout, `setup-node`, `npm ci` and every lint in the job already ran on every such pull request. The four steps simply no longer evaluate a condition that was always true. No step was added, no step made slower, and the job's work is identical. **On a docs-only pull request, nineteen seconds instead of six.** Measured, not estimated, from the green demonstration job below (`10:09:26Z` to `10:09:45Z`): checkout 8s, `setup-node` 2s, `npm ci --ignore-scripts` 3s over the two root dev dependencies (`pg` and `yaml`), and the scan of all 213 files under 1s. Every other step in the job still skips, and all of them together still resolve inside that same second. The `.wolf/buglog.jsonl`-only pull requests described in `.claude/rules/openwolf.md` are on that docs-only path and now pay the same thirteen extra seconds. That is a deliberate trade and worth stating plainly. ## Acceptance, red first The bar on the issue is not "the check runs". It is a deliberate synthetic credential in a docs-only pull request turning the check RED. That demonstration cannot be run on this pull request. The fix edits `.github/workflows/ci.yml`, so this branch can never be classified docs-only, and `on.pull_request.branches` is `[main]`, so a pull request based on this branch triggers no CI at all. The demonstration therefore ran on two throwaway branches: a base branch carrying this exact fix plus a temporary entry in that branches filter, and a head branch whose only change was one planted capture file. Neither the temporary filter entry nor the planted credential appears in this pull request. The planted value was `token=` followed by sixty-two characters of a cycled hex alphabet, the same synthetic constant the linter's own `MUST_CATCH` fixtures use. It was never a real credential, and it was redacted in the very next commit on the same branch. Demonstration pull request: #1418, base `ci/proof-lint-1409-base`, head `ci/proof-lint-1409-red`. Both branches are deleted once this pull request merges, so the run and job ids are recorded here rather than left on a branch, for the same reason visual proof moves to a release. **RED, run [33247080029](https://github.com/sakibsadmanshajib/hive/actions/runs/33247080029), job [99086306257](https://github.com/sakibsadmanshajib/hive/actions/runs/33247080029/job/99086306257)** `Detect changed paths` classified it docs-only, and every heavy job skipped accordingly: ``` verdict: SKIP — every changed path is docs-only web-e2e verdict: SKIP, no changed path can affect the booted browser flow ``` `Repo policy lints (tenant + audit)` then FAILED, on the one step that no longer skips: ``` Run npm run lint:proof-tokens lint-no-token-in-proof-captures: self-test ok (16 assertions) docs/proof/ci-gate-1409/capture-log.txt:11: looks like a real credential (query param, JWT, or API key) — step 3: navigated to http://web-console:3000/invitations/accept?token=0123456789abcdef… Process completed with exit code 1 ``` Job conclusions on that run: `Repo policy lints (tenant + audit)` failure; `Detect changed paths`, `Go tests` (all five modules) and `Web console` success; `rust-tests`, `desktop-tests`, `agent-console-unit`, `docker-build`, `markitdown-sidecar`, `live-integration`, `web-e2e`, `interaction-coverage`, `egress-firewall-e2e` all skipped. That skip list is the proof that the docs-only verdict really was in force while the linter ran. **GREEN, run [33247152597](https://github.com/sakibsadmanshajib/hive/actions/runs/33247152597), job [99086499814](https://github.com/sakibsadmanshajib/hive/actions/runs/33247152597/job/99086499814)** Same pull request, next commit, planted value replaced with `token=REDACTED_INVITE_TOKEN`. Still docs-only: ``` verdict: SKIP — every changed path is docs-only lint-no-token-in-proof-captures: self-test ok (16 assertions) lint-no-token-in-proof-captures: ok (213 files under docs/proof/ scanned) ``` `Repo policy lints (tenant + audit)`: success. So the check is falsifiable on the exact pull-request shape it previously could not fail on, which is the point of #797. Locally, before any push: clean tree `ok (212 files under docs/proof/ scanned)`, exit 0; same tree with the plant, exit 1 naming the file and line. ## Review - **CodeRabbit CLI**: ran, not rate limited. `Review complete, no findings`, one file reviewed. - **Antigravity** (`gemini-3.1-pro-high`, effort high): run with the diff pasted inline, since `agy` executes in its own scratch directory and cannot see this worktree. It quoted this diff's exact lines back, so it reviewed the right change. Two findings, both posted as inline comments and both answered there: ungate `npm ci` since the scanner imports only Node builtins (declined, three measured seconds against a silent invariant), and the unconditional scan blocking an unrelated docs pull request if the corpus already held a leak (accepted as intended behaviour, with the diff-scoped alternative recorded in #1420). ## Rebase note Rebased onto `0d089744a` (#1410, merged) before requesting merge. `.github/workflows/ci.yml` has other editors in flight: #1390 touches object storage, #1410 touched the upstream-refusal classifier. This diff touches only the `repo-policy-lints` job, lines 643 to 660 and 727 to 750, and the rebase was clean. ## Buglog entry ```json {"date":"2026-08-29","title":"lint:proof-tokens skipped itself on the only pull requests that add proof captures","error_message":"Repo policy lints (tenant + audit) reported pass in 6 seconds on PR #1398, which committed 71 proof captures. No step in the job executed.","root_cause":"The changes job in .github/workflows/ci.yml puts docs/ on the inert-path allow-list, so a pull request whose changed paths are all under docs/proof/ sets run=false. Every step in repo-policy-lints was gated on needs.changes.outputs.run != 'false', including npm run lint:proof-tokens, whose scanner reads docs/proof/ and nothing else. The gate therefore disabled itself for its own subject matter, and published a green that a reviewer trusts. Nothing else backstops the text half of a proof capture: GitHub secret scanning, push protection and GitGuardian do not read release assets and read no pixels.","fix":"Run npm run lint:proof-tokens unconditionally, along with the checkout, setup-node and npm ci steps it needs. Every other step in the job keeps its docs-only gate. Rejected carving docs/proof/** out of the classification (would run the whole heavy suite over a screenshot pull request) and a dedicated changes output (copies the scanner's scope into the workflow where it can drift, and cannot cover a capture that landed earlier). Demonstrated red before green on a throwaway docs-only pull request: run 33247080029 failed on a planted synthetic token, run 33247152597 passed once redacted.","tags":["ci","docs-only","proof-captures","credential-leak","unfalsifiable-green","issue-1409","issue-797"],"pr":1417} ``` Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Fixes three defects that together made onboarding a colleague impossible and workspace roles unmanageable.
Closes #534
Closes #535
Closes #536
1. Invite acceptance was impossible for anyone not already signed in (#534)
app/invitations/accept/page.tsxbounced an unauthenticated visitor to/auth/sign-in?next=/invitations/accept, dropping the?token, andlib/auth/next-target.tsallow-listed/invitations/acceptas an exact match only, with no query arm, while/oauth/consentright below it had a prefix arm. The token was lost, acceptance failed, and the error copy told the invitee to request a fresh link, which failed identically. Every invite to somebody without an existing account was a dead end.How the token is preserved without weakening the redirect allow-list:
?tokenbefore the auth check and carries it in thenextvalue throughappendNextParam, so the value stays a URI-encoded relative path./invitations/acceptmoves from the exact allow-list into the prefix arm shared with/oauth/consent. That arm matches only the bare path or the path followed by?, so/invitations/accept?token=...is honoured while/invitations/accept-evil,https://evil.example.com/invitations/accept?token=xand//evil.example.com/invitations/acceptare still refused. Validation is widened by one deliberate entry, never disabled.resolveNextTarget, and the sign-up cross-link plus theemailRedirectToverification target both carry the samenext, so an invitee with no account can sign up and still land on acceptance with the token intact.Token lifecycle is now reported honestly. The control-plane returns a stable machine code per state and the page states the real reason with an action that can resolve it:
invitation_expiredinvitation_already_acceptedinvitation_already_memberinvitation_email_mismatchinvitation_not_foundInternal error text no longer reaches the customer on any of these paths; it is logged instead.
2. Invite success and failure were invisible (#535)
The members page never read its own
searchParams, so the flags its own proxy routes redirect back with were computed and discarded. It now renders a confirmation for a sent invite, a joined workspace and an updated role, and renders a failure as an alert, with the failure winning so a partial success cannot mask it. A member landing on the page also used to hit the console error boundary, because the control-plane restricts the member list to owners and the client threw; the refusal is now an explanation on the page while any other failure still throws so a real outage stays visible.3. RBAC was unmanageable from the UI (#536)
account_invitations.rolefrom themember-only CHECK to('owner','member'). Acceptance now grants the role the workspace chose instead of hardcodingmember.PATCH /api/v1/accounts/current/members/{user_id}.auth.users), and a member with no email on file says so rather than printing a UUID.members.invitepermission.How role changes are authorized: the console decides nothing.
Service.UpdateMemberRoleresolves the caller's own active membership from the database, builds anauthz.Actor(including the platform-admin overlay when wired), and requiresauthz.PermMembersManagethrough the existingauthz.Policyininternal/authz/policy.go. No parallel permission model was introduced. Refusals are ordered so each names its real reason: invalid role, then permission, then unknown target, then the last-owner guard, then the self-change guard.The two invariants:
targetUserID == viewer.UserIDis refused withErrSelfRoleChange(403self_role_change_forbidden), which also covers self escalation by a member, who is refused by the policy before that. Tests:TestUpdateMemberRole_RejectsChangingYourOwnRole,TestUpdateMemberRole_RejectsAMemberEscalatingThemselves,TestUpdateMemberRoleHandler_SelfChangeIsForbidden.ErrLastOwner(409last_owner_required) when the workspace has one active owner, including for a platform admin who holdsmembers.managethrough the overlay, and for the sole owner acting on themselves. Enforced in the service for a precise error and again in theUPDATEstatement, so two concurrent demotions cannot race a workspace into having no owner. Tests:TestUpdateMemberRole_SoleOwnerCannotDemoteThemselves,TestUpdateMemberRole_RejectsAPlatformAdminDemotingTheLastOwner,TestUpdateMemberRole_SecondOwnerMayBeDemoted,TestUpdateMemberRoleHandler_LastOwnerIsAConflict.There is no member-removal endpoint in this codebase, so "removed" reduces to demotion, which the same guard covers.
Tests
Written first, red before green. New Go coverage in
apps/control-plane/internal/accounts/member_roles_test.go(invite role storage and defaulting, invalid role, accepted role granted, unknown/expired/used/already-member/mismatch lifecycle codes, the role-change matrix and both invariants, member email in the listing, malformed member id). New and extended web coverage:lib/members/roles.test.ts,__tests__/members-page-rbac.test.tsx,__tests__/members-role-route.test.ts, plus additions tolib/auth/next-target.test.ts(token query allowed,-evilsuffix and off-site targets still refused),__tests__/invitation-accept.test.tsx(token preserved on both bounces, one distinct message per lifecycle state),__tests__/members-invite-route.test.ts,__tests__/sign-in-next-redirect.test.tsx,__tests__/sign-up-next-redirect.test.tsxand__tests__/auth-routes.test.ts.Verified in Docker per CLAUDE.md:
Visual proof
Captured against a stack built from this branch (control-plane plus web-console containers on their own docker network and ports, browser in its own cookie jar inside that network). Each screenshot carries a capture overlay printing that page's real
window.location.href, so the URL claims are visible in the image. Full index and environment notes:docs/proof/invite-journey-534/README.md.Owner sends an invite with a role, and the confirmation renders:
A signed-out invitee opening the link keeps the token through the bounce:
The sign-up cross-link carries the same token:
Acceptance lands correctly and confirms the join:
The new member appears with a human identity, a role, and a role editor:
A role edit applied through the console:
A refused grant is unmistakable rather than silent, and the member's disabled controls state the real reason:
Each token-lifecycle message, on a genuinely refused attempt:
Two environment boundaries, recorded rather than papered over:
429 email rate limit exceeded) and it rejects several throwaway domains outright. Shot 05 records that refusal. The sign-up half of the round trip is proven up to and including the cross-link and the verification-email redirect target carrying the token, plus theemailRedirectToand/auth/callbackunit tests. The accounts used from there on were created with the admin API, and the sign-in half is proven end to end.Migration
supabase/migrations/20260727_02_account_invitations_owner_role.sqlwidens theaccount_invitations.roleCHECK to('owner','member'), idempotently. Without it the database would reject a co-owner invitation the console can now express.