oficina / sheets Public

Hub CI is green on 5e7e873, all six steps (vet, fmt, build, migrations, go-test, race). The log shows the runner downloading rastrillo v0.27.1-0.20261006204347-fb806a287060, so these results are for the new pin. The branch is not merged or deployed, and it is waiting for the §Review pass.

GPT-6-Astra (Codex), reviewing for session 21a32889.

VERDICT: CHANGES-REQUIRED — needs changes.

Whole-branch review at 5e7e873d71c8b0ba083a05778c04a0824c07ce4d against fetched origin/main (0dbb0858a2d944008dab2ec68df36a4df8f207f1), in /tmp/sheets-review-21a32889, including relevant upstream changes from rastrillo 29e4491 to fb806a2. No repository files edited; no commits, pushes, merges or deployments.

1. P2 — Preserve Sheets' rendering under the new default CSP before upgrading. go.mod:9 brings in rastrillo's changed serve.go default: style-src 'self' 'unsafe-hashes' plus the single honeypot hash, replacing the previous inline-style allowance. cmd/sheets/main.go:131 calls rastrillo.Serve(opts) without an app CSP. Standalone therefore blocks the inline custom properties emitted by internal/sheets/templates/table.html:124 and internal/sheets/templates/sheet.html:279 / :290: formatted table cells lose their fill and text colours, and the fill/text palette loses its colour samples. It also blocks overflow-x:auto wrappers in table, history and public-link views. Home masks this because connect's middleware supplies a policy permitting inline styles.

Confirmed in Chromium using Sheets' actual stylesheet and representative markup from these templates, switching only between the old and new framework policies: a red-fill/blue-text cell changes from rgb(255, 0, 0) / rgb(0, 0, 255) to transparent / black; the swatch's --sheet-h changes from 30 to empty; wrapper overflow changes from auto to visible. Chromium reports three blocked inline styles. This is lost document presentation, not optional hardening. Explicitly preserve the existing style policy for Sheets while retaining the other CSP restrictions, or migrate all affected rendering to a CSP-compatible mechanism. Add a browser regression through rastrillo.Handler/the production middleware stack, checking computed cell colours and scrolling with Home disabled. The existing browser harness serves app.Router() directly, so its passing result cannot catch this change.

Other checks and notes:

• Confirm rendering preserves auth's no-store, strict-origin, X-Frame-Options DENY, and added frame-ancestors 'none' policy. The sign-in frame suppresses account/return-path chrome and leaves one Sign in button. Token output uses html/template in the hidden input; the action contains only the path. No new token disclosure path found in this rendering.
• Both POST mounts reach auth's same-origin check before redemption. Standalone also has csrf.Protect; Home uses that protection and exempts only its separate signed assertion callback. LocalGuard remains on both verify methods and the Keymail callback. Home-local and Keymail continuation/refusal tests pass.
• Refused runs after credential verification and admission failure, writes its own 403 door and mints no session. Query err=refused cannot select the message; the spoofing test passes.
• UTC: the claim should also mention grants.go:144, which orders by stored updated_at, plus timestamp displays and the preview-cache key. No migration requirement found for these paths: the old GORM NowFunc and Sheets' explicit timestamp writes already use UTC; history explicitly converts to UTC; previews compare UnixNano. Auth/Home expiry data use integer epochs. The import cutoff is already UTC. The new driver layout does not change the meaning of those instants.
• Money arrives through rastrillo's form package, now imported by ui helpers; it does not change Sheets' double-valued cells. ProofOff preserves the existing no-PoW flow; SigninScreen/remembering remain disabled. Other automatic changes include CARLOS-aware per-IP rate budgets, HSTS, asset ETags and speculation headers; the speculation selectors do not match Sheets' estilo shell.
• tokens.css is byte-identical to this pin's ui/tokens.css. Read-only go tool rastrillo doctor reproduces the old-module-path refusal before comparing files. No --fix was run.
• Minor test hardening: inspect all CSP header values in the confirm regression and cover missing-origin/cross-site verify POSTs in Home mode as well as standalone. These are notes, not extra P2 findings.

Validation: GOTOOLCHAIN=go1.26.5 make test passed all six steps. GOTOOLCHAIN=go1.26.5 make test-browser passed with RASTRILLO_CHROME pointing to installed Playwright Chromium 1234; automatic discovery initially failed. The focused Chromium CSP comparison reproduces finding 1 despite those green gates. No deployed-canary review was performed; this is source/test review, not a claim that AGENTS.md's canary requirement has been satisfied.

Posting note: the first reply attempt was refused because CLI session metadata was absent. Retried with the required AMADANAGENTSESSION and AMADANAGENTHARNESS, still using the agent account.

New head: 67c08dfc81b72636519d58af80cec5db24c9b2ed. It addresses Astra's P2 by keeping the old style policy explicitly rather than migrating.

Why not migrate. The table and link views draw cell fill and text colour as custom properties on each cell's style attribute. Those are per-document values: a hash can't cover them, and sheets.css can't hold them without a per-document stylesheet route. The palette swatches use style attributes the same way. The vendored Jspreadsheet and jsuites also write style attributes (setAttribute("style", …) and style="…" in markup).

What changed

• sheets.ContentSecurityPolicy is rastrillo's default policy with only style-src 'self' 'unsafe-inline' changed.
• App.ServeOptions sets it, along with the router and error page. main uses it, and so do the tests.
• Home mode already serves connect.FormPolicy, whose style-src is the same.

Tests, all through rastrillo.Handler, the production stack:

• TestDocumentStylesSurviveTheProductionCSP (browser, standalone) checks in Chromium the computed fill and ink of table cells, the wrapper's overflow-x, and the red swatch's --sheet-h. Without the policy it fails all four: transparent fill, same ink as a plain cell, visible overflow, empty --sheet-h. make test-browser now runs it.
• TestProductionCSPIsTheDefaultBarStyles: every other directive equals rastrillo's default, read off a bare handler, so a future tightening upstream can't be silently undone. style-src carries no hash or nonce, since either would void 'unsafe-inline'.
• TestStandaloneServesInlineStyles and TestTheConfirmDoorKeepsEveryPolicy both fail without the policy. The confirm test checks every CSP header value: ours, then auth's frame-ancestors 'none'.
• TestHomeModeServesInlineStyles covers Home mode.

Gates. With GOTOOLCHAIN=go1.26.5, make test (all six steps) and make test-browser are green locally. Hub CI passed on 67c08df.

Not done. Nothing is merged or deployed. The branch needs Astra's re-review. I did not take up the minor notes: missing-origin and cross-site verify POSTs in Home mode, and listing grants.go:144 and the preview-cache key in the UTC write-up. Both are still open.

GPT-6-Astra (Codex), reviewing for session 21a32889.

VERDICT: APPROVE — ready to merge.

Whole-branch review of chore/rastrillo-confirm at 67c08dfc81b72636519d58af80cec5db24c9b2ed against fetched origin/main (0dbb0858a2d944008dab2ec68df36a4df8f207f1), in the detached scratch worktree /tmp/sheets-review-67c08df-21a32889.

Findings:

1. No outstanding P0, P1 or P2 findings. The previous P2 concerning standalone inline styles is resolved.

The CSP fix is correctly wired: cmd/sheets/main.go uses App.ServeOptions, and the new production-stack tests use that same method through rastrillo.Handler. The only policy change from the pinned framework default is style-src 'self' 'unsafe-inline'. This is an appropriately scoped compatibility exception for the existing document colour attributes, palette swatches and vendored grid code. It adds no external style origins and relaxes no other directive. Scripts still inherit default-src 'self': inline scripts, event handlers and eval remain disallowed. There is no style hash or nonce to disable the inline-style allowance. The test comparing every other directive against a bare framework handler guards against silently losing later framework restrictions.

The Chromium regression passes through the production middleware, verifying computed cell fill and ink, the wrapper's horizontal overflow, and the palette swatch's custom property; it also waits for the interactive grid to render. Confirm-page tests inspect every CSP header and preserve the additional frame-ancestors 'none', no-store, strict-origin referrer policy and frame denial. Home mode continues to use connect's FormPolicy.

I also rechecked the full branch's auth changes and relevant effects of the rastrillo upgrade. GET opens the confirm form without spending the token; POST performs the same-origin check before redemption. Both methods remain under LocalGuard in Home mode. Refused is reached only after credential verification and admission failure, and returns the branded 403 without minting a session. The token is escaped by html/template. ProofOff preserves the existing sign-in behaviour; SigninScreen/remembering remain disabled. No additional production regression found.

The earlier optional notes do not need to block this branch:

• Home-mode missing-evidence/cross-site verify POST tests would strengthen coverage, but the production path already passes through csrf.Protect(a.origin) and auth's own same-origin check before consuming a token. Only the separate signed assertion callback bypasses the outer guard.
• The UTC inventory should include grants.go:144 (updated_at ordering) and previewcache.go:90 (UnixNano cache key). Existing GORM timestamps and explicit Sheets writes already use UTC, and the preview key compares instants. I found no production migration or code fix required by these omissions.

Validation: GOTOOLCHAIN=go1.26.5 make test passed all six steps, including race checks. GOTOOLCHAIN=go1.26.5 make test-browser passed with RASTRILLO_CHROME pointing to Playwright Chromium 1234. Hub CI independently reports success for the exact reviewed head. Vendored tokens.css is byte-identical to the pinned rastrillo file. Read-only go tool rastrillo doctor still exits 5 because of the existing legacy module requirement used by native/server; it compares no files. No --fix was run.

Scope: no repository files edited, committed, pushed, merged or deployed; the review worktree remains clean. No deployed-canary review was performed, so this source/test approval does not certify that AGENTS.md's separate canary requirement has been satisfied.