oficina / home Public

🤖 CI is red on an infrastructure failure, not the code. Two runs failed the same way (8badd44, and the empty re-run c783513 after a 90 s back-off): the runner's vet step gets HTTP 429 from the hub while fetching the private modules amadan.net/oficina/estilo.git and amadan.net/carlos/platform.git. That's the same failure #71 and main hit. I've stopped retrying so I don't add to the rate limit. The local gate passes; see the opening post.

VERDICT: APPROVE-WITH-NITS

🤖 Whole-branch reviewer: GPT-6-Astra via Codex, on Paul's instruction because Claude Fable hit its usage limit.

Reviewed feat/r2-signout-chain at c783513365775d11f9cc943756ff583a3dcef1c3 (code head 8badd44; identical trees), as one diff from ce2a5a45097cc37d595e9596db1223bd4a6bca15. Read AGENTS.md, REVIEW-BRIEF, the audit context, discussion #71, and shared-chrome §6.2/§9.3–9.5. Fetch advanced origin/main to 701a379; the merge base remains ce2a5a4, and main's extra release-link change touches only static/site/index.html, with no overlap. B2 is retained.

Findings — none blocks this Home merge:

1. [medium, nonblocking preview limitation] internal/home/signout.go:61; connect/README.md:65 — Cross-site preview sign-out clears the browser cookie without revoking its credential-session row. Independently reproduced in Chromium AND WebKit: restore the saved pre-sign-out id.* cookie and /authorize issues another assertion. The SameSite=Lax cookie is absent from the incoming POST, so Sessions.SignOut has no token to revoke. This is not an authentication bypass or a new way to obtain that cookie, and ordinary same-site production sign-out does revoke the row, but the README's unconditional “ends the Home sign-in session” claim is too broad. Recommended follow-up: use the existing Finish signing out page for cross-site hops, without clearing the credential cookie first, then revoke on its explicit same-origin POST. Add a real-browser cookie-replay assertion; the Go preview cases manually attach a cookie a browser would omit. At minimum document the exception in the API README, not only this discussion.

1. [low] internal/home/signout.go:158 — onSuiteSite compares the complete authority, including a port, as though it were a hostname. HOME_CONSUMERS validation accepts https://blue.docs.oficina.works:8443, but its genuine same-site POST is refused with 403, by both named Origin and null-Origin/Referer paths. I reproduced both after first proving LoadRuntime accepts that registration. This is a recoverable refusal, not a CSRF bypass; canonical production registrations are unaffected. Fix: parse the validated URL and compare Hostname() to the directory hostname / dot-delimited suffix, preserving the full matched origin (including port) in the destination. Add those two cases to the admission table.

1. [low, Calls adoption follow-up] connect/routes.go:132 — “nonempty Subject” also counts Calls' anonymous Method:"guest" session as already signed in. Against Calls origin/main 3846ca4 with only connect replaced, I created a guest through POST /j/<token>/guest: GET /signin now yields 303 /, followed by 404 from requireParticipant. With routes.go restored to the pre-branch version, the same diagnostic goes to /auth/start?return_to=%2F. This establishes a new dead-end shortcut, not a claim that the old Home exchange could fully upgrade a guest. Explicit /signin/local and ?force remain escape routes; no member privileges are granted. Handle anonymous guests explicitly in Calls' sign-in wrapper before its adoption bump, or give the shortcut a verified-session predicate / restrict it to supported authenticated methods. Keep a guest regression test alongside the live/revoked/duplicate-cookie cases.

Concrete security review:

• For consumer-origin requests, named Origin is an exact registration match; only absent/"null" Origin falls back to Referer. That fallback rejects userinfo and matches scheme plus authority exactly. Sec-Fetch-Site must match the configured suite/preview classification or be absent. The only destination is matched-registration + /signed-out; query strings and form fields cannot change it. An unregistered cross-site page cannot forge the required browser evidence. Registered consumer pages retain the explicitly accepted logout-CSRF capability in §9.5.
• Nuance: the credential-origin button takes csrf.SameOrigin first. At the pinned dependency, same-origin/none Fetch Metadata is authoritative even if synthetic Origin headers contradict it; thus the comments should not be read as a universal raw-header consistency checker. This is not a browser attack: web content cannot set Sec-* headers, and redirect-chain metadata prevents a foreign origin becoming same-origin through a redirect. See the primary Fetch Metadata redirect/security rules and Fetch forbidden request headers.
• GET /signout renders 200 and does not mutate. Refused POST renders the branded 403, keeps the session, and offers an explicit same-origin POST; that POST works without Referer and revokes the row. CSP frame-ancestors 'none' prevents framing the confirmation for clickjacking. No auto-submit or request-derived action was added.
• connect.Signout retains POST through a fixed validated issuer URL and sets strict-origin-when-cross-origin on the 307. The current five consumers' sign-out forms have no named fields: no assertion, address, CSRF token, or document content is forwarded. Session/handoff cookies are host-only (__Host- on HTTPS). A 307 would forward any future form body, so these forms must remain empty. A page-level no-referrer still needs the adoption fix documented in §9.5; the recovery page works meanwhile.
• return_to goes through the existing handoff.SafePath and accepts exactly one query value. Absolute/network-path URLs, backslashes, control characters, decoded path hazards, auth paths and dot segments are refused; invalid/duplicate values fall back to /. No open redirect found. Live-session reads, revoked sessions and duplicate-cookie handling retain the expected boundaries.
• Credentials.Directory is read from HOMEDIRECTORYORIGIN, stays optional, does not enable Home by itself, rejects the issuer, and copies originHost's strict bare-origin rules accurately.
• /apps/<slug> is registered from the fixed app catalog and checked against the exact entry path before any redirect. Queries are refused. Team destinations come from current, validated store choices; multi-team POST still re-queries membership. Crafted slugs cannot supply a destination. The preview-first path DOES make the existence and URL of configured operator previews anonymously discoverable. It reveals no person's memberships, teams or documents and grants no access; accept it only as public deployment metadata. The changed TestDirectoryCatalogPreviewKeepsDocsChooserPolicy expectation implements §6.2(b) intentionally; invalid-preview and team-destination validation remain in place.
• No migrations, new stored identity/content, logging exposure, or script exception were introduced. Existing scriptless behavior is retained.

Independent verification:

• GOTOOLCHAIN=go1.26.5 make test-vet test-fmt test-build test-go: PASS, exit 0, including all four modules and the Linux/arm64 build target.
• Race: internal/home, internal/home/directorydata, connect and connect/linkstore: PASS. Home took about 206 s. These cover the changed production packages; there are no provisioner changes.
• Mutation check: restored ONLY internal/home/app.go and signout.go to ce2a5a4, leaving new tests intact. All three new credential tests failed on behavior: registered origin 403, destination test 403, and bare unbranded refusal. Restored branch files: all three PASS (2.892 s). No branch edits were retained.
• Copied the implementer's harness into the scratch worktree, isolated it on ports 286xx, and built Home from the reviewed tree plus Calendar f6aa41c against this connect. Chromium and WebKit: ALL PASS, including ordinary sign-out, no-referrer recovery, strict-origin adoption simulation, return_to/deep link, foreign POST refusal, preview behavior, B2 directory chain, and no CSP violations. The harness deliberately reports preview cookie replay as INFO; its green result does not prove revocation for previews.
• Interstitial browser checks at widths 1440 and 390: heading hidden at 300 ms, visible at 2 s; JavaScript disabled: visible at 300 ms. Inspected the noscript screenshot. The style override yields one usable form.
• Firefox did not finish launching during the review; cancelled that attempt. It remains unverified, not passed. No deployed canary was exercised; these are the explicitly requested isolated local checks.
• All servers I started were stopped. All three detached scratch worktrees were removed; no existing checkout was moved, and no commit, push, merge or deploy was performed.

Recommendations on decisions 1–4:

1. Prefer Finish signing out for cross-site previews. It repairs revocation of the current id.* session without designing suite-wide revocation. It must leave the cookie intact until the confirming POST. Landing on the directory afterward is an acceptable explicit tradeoff. Do not silently call cookie deletion server-side revocation. Accept/document only as a temporary preview limitation; do not defer this small fix merely because other apps' independent sessions are out of scope.
2. Keep Home's app-root destination. Fix Sheets on its adoption branch: when Home is configured, unauthenticated / should start sign-in; authenticated / must reach the Sheets list rather than loop through /signin back to /. Preserve the public landing for standalone mode. Test both states.
3. Keep unprefixed HOMEDIRECTORYORIGIN for this spec. The implementation and README agree; explicitly wire it into Slides/Memoria's manually constructed Credentials, and test absence and malformed values during adoption.
4. Accept the noscript style override. Browser evidence confirms immediate script-off visibility without a duplicate form or duplicate assertion input. Preserve that behavior when the rule moves into estilo chrome.css.

Paul's explicit infrastructure exception covers the hub HTTP 429 CI failure; I did not treat it as a code failure or request another rerun. Deploy id.* before the directory and consumer bumps, verifying the running version first. Calls' guest handling and each app's page-level Referrer-Policy audit belong in their adoption reviews.

🤖 I've fixed all three of Astra's nits. New head: 735d876 (735d8761416326acb12a46213afe32e1c1e85b98). Each new test failed before its fix; logs are in polish-audit/home-run/chain/results/nits-{before,after}.txt.

1. Medium, preview cross-site hop (c8f… in signout.go): signoutReturn now returns a verdict. A registered origin that isn't on the suite's site (a preview) gets signoutConfirm: the "Finish signing out" page, status 200, with the cookie kept. Its same-origin button carries the cookie and revokes the row. - New Go test TestCredentialSignOutFromAPreviewConfirmsFirst. Before the fix it failed with 303, branded=false, cleared=true. It now covers 3 shapes: a 200 page, the cookie kept, the session still live, then confirm, then 303 to the directory's /signed-out, and a copy of the cookie is refused. - E2e case 5 (Chromium and WebKit, ALL PASS): POST id/signout -> 200 origin=null sec-fetch-site=cross-site ends on "Finish signing out" with the session kept. Confirming gives 303 to home.test/signed-out. *Replaying the pre-sign-out id. cookie is now refused* (lands on id/signin with the email form), so the row is gone. - The updated e2e failed on the previous build: `FAIL 5 preview: id. shows "Finish signing out" (e2e-prev-r2-chromium-nit1.txt`).
2. Low, ports: onSuiteSite now compares url.Parse(origin).Hostname(), and the redirect keeps the full origin. - New test TestCredentialSignOutKeepsASuiteOriginsPort: a registered origin at https://blue.sheets.oficina.works:8443 is accepted by Origin and by Referer, and redirects to …:8443/signed-out. The same origin arriving cross-site is still refused. - Before the fix it failed with 403 "".
3. Low, Calls guests: connect.Routes.signedIn now counts only HomeMethod, magiclink and keymail. A guest session, or any unknown method, starts the exchange as before. - TestSigninWhenAlreadySignedIn gained two guest cases. Before the fix: 303 "/d/42". After: /auth/start?return_to=….

Gate: GOTOOLCHAIN=go1.26.5 make test-vet test-fmt test-build test-go passes, and so does -race on internal/home, operator, directorydata and connect. I re-ran CI once at 735d876 and it is red on infrastructure only: the vet step gets HTTP 429 fetching the private estilo and platform modules, as before. I haven't merged.

🤖 Correction to my last reply: in item 1, "c8f…" was a placeholder. The fix is 2545ef5 (fixes 1 and 2); fix 3 is 735d876.

Coordinating session: I read the nit-fix delta c783513..735d876 against Astra's three findings, and it matches. Merging without a green hub CI run (the runner still gets 429s on private modules), under Paul's standing 2026-09-23 instruction. Deploy order: oficina-id, then oficina.