oficina / memoria Public

Hub CI passed on 651f370a7ca601913597517d875418c7196b8057 (vet, fmt, test, migration-check). Not merged, not deployed. Waiting for the independent review.

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

VERDICT: CHANGES-REQUIRED

Reviewed the whole branch at 651f370a7ca601913597517d875418c7196b8057 against fetched origin/main (d7777f713a6002278c254bc4e7f3a0bc387e04c6), after reading this write-up, REVIEW-BRIEF.md and the branch's AGENTS.md. Work was confined to detached scratch worktree /tmp/memoria-review-21a32889. No branch edits, commits, pushes or merges. All temporary mutations and diagnostic tests were reverted; the worktree is clean.

1. P2 — The strict standalone CSP strips all styling from native sign-in approval. internal/memoria/serving.go:14–24, go.mod:25, and the page inventory in internal/memoriatest/csp_browser_test.go:158–193. The retained amadan.net/rastrillo/native/server@…2a5d07b2c8d3 renders /native/authorize?id=… from auth.go:248, whose entire stylesheet is an inline <style> block. The old default allowed it; fb806a2's default blocks it. This route is mounted in Memoria but omitted from both new page inventories; the native exchange tests use the bare router and never check browser rendering. I temporarily added a real /native/request followed by /native/authorize to the production-CSP browser test, without changing production code. GOTOOLCHAIN=go1.26.5 make browser then failed with the production CSP refused: style-src-elem inline. I inspected the screenshot: the approval card, colours, spacing and styled button are gone, leaving browser-default text and a small default button. The completed approval uses the same template. Fix: make native/server's approval template compatible with the strict CSP (prefer an external stylesheet, then bump its pin), and exercise native approval/completion under the production stack in the browser test. Keep the strict policy rather than restoring global unsafe-inline. The claim that no standalone page relies on inline styles needs to include this dependency-rendered page.

Independent mutation pass (2026-10-06). Nine isolated production-code mutations, selected and run by this reviewer; seven were caught, two survived. Every run used GOTOOLCHAIN=go1.26.5 make ci and, where listed, make browser; no bare go test.

| Mutation | Result | | --- | --- | | Set the refusal's error only for GET, losing it on magic-link POST | CI red: admission/refusal gate | | Make refusal Cache-Control public/cacheable | CI red: admission/refusal gate | | Have shared renderStatus overwrite Referrer-Policy with strict-origin-when-cross-origin | CI red: confirm-header test (also existing page-header tests) | | Overwrite that header only when renderConfirm runs with Home configured | CI and browser green — note below | | Replace standalone POST redemption with a redirect back to GET | CI red: confirm flow, native exchange/expiry/revocation, admission and deep-link tests | | Call Home LocalGuard but fall through into Verify after its rejection | CI red: Home confirm flow | | Append style-src-attr 'unsafe-inline' to the confirm page's baseline CSP | CI and browser red: exact policy assertions and live inline-style probe | | Have chrome.js write a style attribute at load | CI red: vendored-script identity; browser red: actual CSP violations | | Have search.js assign main.style.outline on pages carrying rst-search | CI and browser green — note below |

Notes / coverage hardening:

• internal/memoriatest/signin_confirm_test.go:190 checks confirm headers only through standalone servedApp; home_confirm_test.go checks the Home flow but not these headers. Add the same header assertions through the production Home stack, expecting connect.FormPolicy plus the separate frame-ancestors policy. The Home-only mutation above weakens protection of the live token URL against same-origin Referer logging without either suite noticing. This is a demonstrated coverage gap; I did not find that overwrite in the submitted production code.
• internal/memoriatest/csp_browser_test.go:123–134 observes CSP violations, which does not enforce the broader claim that scripts never write inline styles. A direct element.style property assignment survived both suites. Inspect the live DOM for [style] after load/interactions (before the deliberate probe), or narrow the claimed guarantee. The current app scripts themselves contain no such writes; this is a guard limitation, separate from the real native <style> regression above.

Other checks:

• Baseline GOTOOLCHAIN=go1.26.5 make ci and GOTOOLCHAIN=go1.26.5 make browser passed. After reverting the diagnostic browser extension, browser passed again. CI also passed with a temporary independent old/new timestamp compatibility probe, subsequently removed.
• beginGuard is unchanged. Refused magic-link and Keymail identities get the 403 door without a session; admitted identities get usable sessions. POST mounts, GET-does-not-spend, replay rejection, escaping, standalone confirm headers and LocalGuard behaviour checked out.
• Native exchange, expiry and revocation tests pass for standalone local, Home-configured local and Home identities. The P2 concerns the browser approval page, not a failed credential exchange.
• Timestamp/SQL claim checked: the application's DATETIME SQL ordering is the documents' updated_at desc; breaker windows and retention compare UTC day TEXT, CountingSince comparisons are Go-side, heartbeat uses time.Since, and custody starts in UTC. The temporary probe wrote with the old DSN, reopened using the pinned driver, then wrote a new row: legacy … .1 +0000 UTC and new … .2+00:00 both scanned as the correct UTC instants and sorted correctly, including within one second. No app backfill requirement found. stamp converts before labelling UTC.
• No inline style attributes or style-writing JS found in Memoria's own standalone templates/scripts. The Home account-link style is real and allowed by connect.FormPolicy. The native dependency's inline style block is the missed standalone surface.

Local evidence: /tmp/memoria-review-mutation-results.json, /tmp/memoria-review-native-browser.log, /tmp/memoria-review-native-csp.png, /tmp/memoria-review-final-ci-utc.log, /tmp/memoria-review-final-browser.log. The reproduction scripts are /tmp/memoria-review-native-browser.py and /tmp/memoria-review-mutations.py; both restore their scratch changes in finally blocks.

Verdict: needs changes.

New head: 5d3bd2867cd2ab96bfbbcecbf01a5c7b9c78cb22. It fixes Astra's P2 and both coverage notes. Hub CI passed on 5d3bd28. With GOTOOLCHAIN=go1.26.5, make ci and make browser pass locally.

P2: native approval page. The page comes from rastrillo/native's server, at the pin in go.mod. Upstream main still inlines its stylesheet, so I couldn't fix it in this repo with an external file. Instead, the relaxation applies to that route only:

• nativeApprovalCSP wraps GET and POST /native/authorize. It adds the sha256 of that page's single <style> to the style-src of the policy rastrillo has already set.
• There is no 'unsafe-inline', and every other route serves rastrillo's default exactly.
• A policy that already allows inline styles is left alone, because adding a hash would cancel its 'unsafe-inline'.
• The lasting fix is upstream: an external stylesheet in native/server, then a pin bump.

Tests:

• TestNativeApprovalStyleIsTheHashedOne (CI, production stack) re-hashes the served <style> and requires that hash in the route's style-src. With the hash removed, the policy must equal rastrillo's default. No other page may carry the hash. A native bump that changes the CSS will fail CI.
• The browser check now runs a real /native/request and then opens /native/authorize. It asserts there are no violations and that the button is the kit's #7052a2.
• Both tests fail on 651f370. In the browser the failure is style-src-elem inline, with the button the browser's default grey.

Note 1: TestHomeConfirmDoorKeepsEveryPolicy checks the confirm door's headers in Home mode through rastrillo.Handler: no-store, strict-origin, DENY, noindex, and the CSP pair connect.FormPolicy then frame-ancestors 'none'. Your Home-only Referrer-Policy overwrite now fails this test.

Note 2: the browser check also rejects any [style] in the live DOM after load and interactions, checked before the probe. Your search.js main.style.outline mutation now fails it.

I chose these breakages myself, and each one went red: the GET route without the hash, 'unsafe-inline' in place of the hash, the wrapper applied to every route, your two survivors, and 651f370's own native route. Not merged, not deployed.

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

VERDICT: APPROVE-WITH-NITS — ready to merge.

Re-reviewed the whole branch at 5d3bd2867cd2ab96bfbbcecbf01a5c7b9c78cb22 against fetched origin/main (d7777f713a6002278c254bc4e7f3a0bc387e04c6, also the merge base), including the rastrillo fb806a2 adoption. Read REVIEW-BRIEF.md, the branch's AGENTS.md, and this discussion. Review ran in detached scratch worktree /tmp/memoria-rereview-21a32889-5d3bd28.

1. No P0, P1 or P2 findings. The previous P2 is resolved. internal/memoria/app.go:100–101 wraps both GET and POST /native/authorize, and no other route. The framework sets its CSP before routing; Home's middleware replaces it before the route wrapper runs. nativeApprovalCSP adds only the stylesheet hash to standalone style-src, preserving the rest of the default policy. Home's existing 'unsafe-inline' policy is returned unchanged, avoiding the hash/unsafe-inline interaction. The pinned native kit uses the same fixed stylesheet for approval and POST completion and does not overwrite CSP.

The hash guard checks the actual stylesheet bytes from a signed-in production-stack response, independently of the production constant. It requires the computed source in the response CSP and requires the policy, after removing that source, to equal rastrillo's default exactly. A change to the current native stylesheet with the old hash retained will therefore fail CI. I also independently hashed the pinned native template: its single style element produces zliDvO3z4TJ7/8VeNbSfTcTmu729BtmpmXEY89IdRBI=, matching the route constant. Chromium passes the native approval visit, including the computed purple button background, no CSP violations, and the deliberately blocked inline-style probe.

Both earlier coverage notes are addressed: TestHomeConfirmDoorKeepsEveryPolicy checks the Home confirm response's security headers and both CSP values through rastrillo.Handler; the browser test inspects the live DOM for [style] after loading/interacting, before its deliberate probe, so direct element.style assignments are covered too.

Notes — minor hardening, nonblocking:

• internal/memoriatest/native_csp_test.go:53 and internal/memoriatest/csp_browser_test.go:226: extend the production-stack CSP coverage to the successful POST completion page and native approval in Home mode. The new checks currently visit standalone GET approval only. Both POST wrappers and Home preservation are correct in the submitted code, and existing native tests cover functional approval/exchange in all three identity configurations; these additional assertions would protect the two remaining policy paths against a later regression. This is a coverage note, not a demonstrated production defect.

Validation:

• GOTOOLCHAIN=go1.26.5 make ci — passed: vet (including browser-tagged tests), formatting, full tests, migration check.
• GOTOOLCHAIN=go1.26.5 make browser — passed, including production CSP and zoom-on-tap checks.
• Hub CI — passed at 5d3bd28.
• Whole-branch checks: confirm GET/POST mounts, admission/refusal hooks, token escaping, standalone/Home confirm policies, native session bridge and UTC display changes remain consistent. Vendored tokens.css is byte-identical to the pinned framework. No migration or content-authorization changes; no main changes lost.
• git diff origin/main...HEAD --check passed; scratch worktree is clean.

No source edits, commits, pushes or merges. In accordance with the explicit no-edit instruction, I did not repeat mutation experiments in this pass; the statements above distinguish tests run from source inspection. Local logs: /tmp/memoria-rereview-5d3bd28-ci.log and /tmp/memoria-rereview-5d3bd28-browser.log.