Backport browser CSRF protection and gate automatic production rollout #19

Merged

erik merged 5 commits from task-c6c678da-csrf into main 2026-10-05 15:04:29 -05:00

Conversation 1 Commits 5 Files changed 41 +1622 -232

erik commented 2026-10-05 13:34:55 -05:00

Owner

Copy link

Summary

Task: task-c6c678da — Roll out Slugkit CSRF protection.

• Backport Slugkit PR #298 / merge 4853d91b0a32b4a58eaf44e264ae929964bd9c8a instead of replacing the site template, preserving branding, account/actor customizations and the existing federation TLS-proxy fix.
• Adopt shared expiring session-bound CSRF tokens, strict operator-owned Origin validation and signed anonymous login context before browser writes/uploads, including multipart forms and passkey fetches. Preserve user/role checks, cookie flags, bearer API/OpenAPI and federation behavior.
• Remove legacy /cli/auth entirely; make GET logout non-mutating and retain protected POST logout plus the single-use email callback exception.
• Keep integration verification local-only: CI runs deterministic test:ci; release workflow no longer provisions a smoke-test container. Local image validation remains mandatory before publication and refuses CI execution.
• Record adoption provenance and the detailed implementation/deployment/backup/rollback/public-verification plan in docs/csrf-rollout.md; strengthen release approval to explicitly cover automatic production deployment on Docker Hub publication.

Verification

• Build, lint, whitespace checks: pass; generated CSS unchanged.
• 437 local tests across 50 files, including existing federation integration.
• 361 deterministic CI tests across 43 files, excluding integration files.
• 75 focused CSRF cases cover complete mutation inventory, Origin/token/session/context attacks and rotation, no-write/no-upload assertions, proxy/origin/WebAuthn/email behavior and token-free bearer API operations.
• 34 local browser checks, including seven synthetic CSRF workflows, actual virtual-authenticator registration/login/deletion, user/role/deactivation, multipart actor upload, email callback, setup, issuance/revocation/logout and actual ambient-cookie cross-Origin attack.
• Local production image (proposed version 0.3.12, not published) passes public health, API health and the customized homepage checks; disposable fixture container removed.
• Local logs: /tmp/evc-csrf-{build,lint,local-tests,ci-tests,browser,image-build,image-check}.log.

Production gates — intentionally still pending

This implementation PR does not complete the rollout task. Public HTTPS health currently reports 0.3.11; operator confirmed that Docker Hub publication auto-deploys production. No release tag, image publication, runtime configuration change, production mutation or deployment occurred.

Before publication, require operator-owned evidence of actual proxy/backend topology, stored site URL and WebAuthn identity, replicas/restarts, signing-key secret-management mechanism, backup/restore and auto-deploy rollback control. A secret must only be provisioned through operator-owned management and never exposed in logs/source/chat. Require separate explicit production deployment approval and approved synthetic verification scope. Perform safe actual public positive/negative verification only after that approval. Task closure needs its own approval/close gate after rollout.

Related security-header rollout task-dae6192b remains active and unchanged. This PR does not adopt header middleware/nonces or enable HSTS; later header adoption must retain token/meta/fetch wiring and avoid conflicting proxy/app policies. No CLI release, dependency audit remediation, OpenAPI changes, migration or legacy compatibility shim is included.

## Summary Task: `task-c6c678da` — Roll out Slugkit CSRF protection. - Backport Slugkit PR #298 / merge `4853d91b0a32b4a58eaf44e264ae929964bd9c8a` instead of replacing the site template, preserving branding, account/actor customizations and the existing federation TLS-proxy fix. - Adopt shared expiring session-bound CSRF tokens, strict operator-owned Origin validation and signed anonymous login context before browser writes/uploads, including multipart forms and passkey fetches. Preserve user/role checks, cookie flags, bearer API/OpenAPI and federation behavior. - Remove legacy `/cli/auth` entirely; make GET logout non-mutating and retain protected POST logout plus the single-use email callback exception. - Keep integration verification local-only: CI runs deterministic `test:ci`; release workflow no longer provisions a smoke-test container. Local image validation remains mandatory before publication and refuses CI execution. - Record adoption provenance and the detailed implementation/deployment/backup/rollback/public-verification plan in `docs/csrf-rollout.md`; strengthen release approval to explicitly cover automatic production deployment on Docker Hub publication. ## Verification - Build, lint, whitespace checks: pass; generated CSS unchanged. - 437 local tests across 50 files, including existing federation integration. - 361 deterministic CI tests across 43 files, excluding integration files. - 75 focused CSRF cases cover complete mutation inventory, Origin/token/session/context attacks and rotation, no-write/no-upload assertions, proxy/origin/WebAuthn/email behavior and token-free bearer API operations. - 34 local browser checks, including seven synthetic CSRF workflows, actual virtual-authenticator registration/login/deletion, user/role/deactivation, multipart actor upload, email callback, setup, issuance/revocation/logout and actual ambient-cookie cross-Origin attack. - Local production image (proposed version `0.3.12`, not published) passes public health, API health and the customized homepage checks; disposable fixture container removed. - Local logs: `/tmp/evc-csrf-{build,lint,local-tests,ci-tests,browser,image-build,image-check}.log`. ## Production gates — intentionally still pending This implementation PR does not complete the rollout task. Public HTTPS health currently reports `0.3.11`; operator confirmed that Docker Hub publication auto-deploys production. No release tag, image publication, runtime configuration change, production mutation or deployment occurred. Before publication, require operator-owned evidence of actual proxy/backend topology, stored site URL and WebAuthn identity, replicas/restarts, signing-key secret-management mechanism, backup/restore and auto-deploy rollback control. A secret must only be provisioned through operator-owned management and never exposed in logs/source/chat. Require separate explicit production deployment approval and approved synthetic verification scope. Perform safe actual public positive/negative verification only after that approval. Task closure needs its own approval/close gate after rollout. Related security-header rollout `task-dae6192b` remains active and unchanged. This PR does not adopt header middleware/nonces or enable HSTS; later header adoption must retain token/meta/fetch wiring and avoid conflicting proxy/app policies. No CLI release, dependency audit remediation, OpenAPI changes, migration or legacy compatibility shim is included.

erik added 2 commits 2026-10-05 13:34:55 -05:00

fix: adopt upstream browser CSRF protection before site rollout b01eff3991

docs: normalize backported security guidance whitespace 608e9c2776

erik commented 2026-10-05 13:38:39 -05:00

Author

Owner

Copy link

Review update

• PR: Forgejo #19, open; reviewed head 608e9c27766ac7463365cc3200b201f653296353.
• Task: task-c6c678da — Roll out Slugkit CSRF protection.
• Result: changes-requested under the shared review logic: acceptance criterion 7 is not-evident. This is an implementation PR for a rollout task, not evidence that production is protected. Pending production gates are intentional, not authorization to bypass them.

Summary and security/quality assessment

• Independently reviewed the complete PR diff, relevant changed application/workflow files, rollout plan and existing verification logs; loaded the task once. No tests, builds, browser/container checks or production inspection were performed by this reviewer.
• Scoped adoption records Slugkit merge 4853d91b0a32b4a58eaf44e264ae929964bd9c8a; site branding, actor/profile forms and federation implementation are retained. Task comments and rollout documentation report the upstream prerequisite completed; this review did not independently re-query upstream.
• Shared middleware precedes the setup gate/routes. Session-bound HMAC tokens, signed anonymous contexts, exact trusted-Origin checks, authoritative header handling and duplicate/non-string form rejection protect the inventoried mutations. Authorization headers cannot bypass browser protection. Existing session/role checks remain. Multipart parsing precedes token validation but object-storage/business writes do not. Request-local token rendering, hidden fields and passkey fetch headers are wired; protected responses use no-store.
• Legacy CLI route/template/registration are deleted; GET logout redirects without deleting sessions/cookies, protected POST deletes the valid session. The single-use magic-link callback exception is preserved. API/OpenAPI/federation contracts are not modified by the diff; bearer operations stay outside the browser guard.
• No concrete newly introduced correctness defect identified in the inspected implementation. This does not establish live production topology, cookie behavior, identity alignment or protection.

Acceptance criteria

Statuses apply to the original rollout task, not a reduced implementation-only substitute. Local evidence is identified explicitly.

1. met — Explicit scoped backport, upstream commit and site-owned route/form customization recorded in .slugkit-site.json and docs/csrf-rollout.md. This describes repository adoption, not deployment adoption.
2. partial — Local deterministic tests cover the complete 16-route mutation inventory, invalid/missing/cross-origin/expired/wrong-binding submissions and no-write/no-upload assertions; local browser coverage exercises legitimate account/user/actor/setup/key/passkey/email/logout flows and an ambient-cookie sibling-Origin attack. Equivalent safe public verification is still pending.
3. met — /cli/auth is removed without redirect/shim; GET logout is non-mutating; current credential guidance uses /settings/api-keys and API-only CLI login. Supported by implementation and local regression evidence, not a claim about the currently deployed image.
4. partial — Origin precedence, forwarding-header rejection, cookies, session/context/key rotation and replica/restart behavior are implemented/documented and tested locally, with no HSTS adoption. Actual proxy/backend topology, stored production site URL/WebAuthn/email identity, replicas and operator-owned signing-key mechanism remain unverified/unapproved.
5. met — Current bearer API/CLI contract and federation implementation remain unchanged and browser-token-free in the diff; token-free bearer auth/write tests and existing local compatibility suite pass. No live federation testing or CLI release is claimed.
6. met — Existing logs show build/lint success, 437 local tests/50 files, 361 deterministic CI tests/43 files, 34 browser checks, and local proposed-version 0.3.12 image build/validation. /tmp/evc-csrf-ci-evidence.txt records successful Forgejo job #66 on the exact reviewed head. CI uses test:ci; release runtime smoke execution is removed; browser/federation/runtime testing stays local-only, with workflow-policy tests. Logs are historical evidence, not freshly rerun checks or public deployment verification.
7. not-evident — No explicit human production deployment approval, publication/deployment result or actual public positive/negative verification exists. Operator confirmed Docker Hub publication auto-deploys production; no deployment, runtime configuration or backup changes were approved/performed. Local checks cannot satisfy this criterion.
8. partial — Reviewed plan records provenance, local evidence, consistent SQLite/media backup approach, restore/pause/pin prerequisites, immutable-image rollback and safe synthetic public checks. Header task task-dae6192b remains active/unchanged; nonce/middleware coordination and separate HSTS decision are documented. Actual backup/restore readiness, auto-deployer rollback control and rollout evidence remain outstanding.

Counts: 8 total; 4 met; 3 partial; 1 not-evident. criteria_missing=4 means all criteria not fully met (partial plus not-evident).

Blocking issues (2)

1. Operator prerequisite evidence missing (criteria 4/8): Before publication, obtain sanitized evidence of canonical HTTPS/proxy/backend/Host behavior, stored site URL and identity alignment, replica/restart strategy and secret-management mechanism; identify the current immutable image and deployment configuration, recoverable consistent DB/media backups and verified restore/auto-deploy pause/pin rollback control. The plan correctly requires approval before any runtime/secret/backup change; proposed instructions are not proof of readiness. Never expose secret values or inspect unrelated personal records.
2. Approved rollout and public verification absent (criteria 2/7/8): Complete the explicit human merge gate and prerequisite review, then obtain separate explicit production deployment approval covering auto-deploy publication plus any approved configuration/secret/backup operations and synthetic verification scope. Only after that approval may deployment and safe actual public positive/negative checks occur. Record sanitized deployment/version/digest, workflow, verification and limitations, then rerun the review/closure gates. This review does not direct unauthorized deployment and does not close the task.

Warnings (1)

1. Existing image-build installation output reports 8 runtime dependency vulnerabilities (6 moderate, 2 high) and 17 total including development dependencies (8 moderate, 9 high). Runtime versions are unchanged by this backport, but the supplied logs do not establish advisory applicability or remediation. Treat this as a non-blocking implementation warning, not proof of deployment safety: triage and record any deployment-blocking advisory before seeking release approval; do not apply broad audit fixes implicitly.

Deployment-plan disposition and final verdict

• The plan correctly separates merge from publication/deployment approval, accounts for auto-deploy, avoids copying a live WAL database alone, requires controlled rollback, and warns that returning to 0.3.11 restores CSRF exposure. Safe synthetic verification and separate approval for destructive restore/rotation are appropriate. Operational evidence and approvals remain pending; the plan is not an executable production authorization yet.
• Final verdict: changes-requested because the full task has a not-evident acceptance criterion and partial operational gates, despite substantial locally verified implementation work. Required next steps are the operator evidence and explicit approval sequence above, followed by authorized rollout/public evidence and renewed review. Do not relabel these criteria met from local logs, implicitly complete the header task, publish a CLI/image release, merge, or close this task.
• Human merge decision remains pending; this review grants neither merge approval nor production approval.

### Review update - PR: [Forgejo #19](https://forge.caradoc.com/erik/erikvancraddock.com/pulls/19), open; reviewed head `608e9c27766ac7463365cc3200b201f653296353`. - Task: `task-c6c678da` — Roll out Slugkit CSRF protection. - Result: **changes-requested** under the shared review logic: acceptance criterion 7 is **not-evident**. This is an implementation PR for a rollout task, not evidence that production is protected. Pending production gates are intentional, not authorization to bypass them. #### Summary and security/quality assessment - Independently reviewed the complete PR diff, relevant changed application/workflow files, rollout plan and existing verification logs; loaded the task once. No tests, builds, browser/container checks or production inspection were performed by this reviewer. - Scoped adoption records Slugkit merge `4853d91b0a32b4a58eaf44e264ae929964bd9c8a`; site branding, actor/profile forms and federation implementation are retained. Task comments and rollout documentation report the upstream prerequisite completed; this review did not independently re-query upstream. - Shared middleware precedes the setup gate/routes. Session-bound HMAC tokens, signed anonymous contexts, exact trusted-Origin checks, authoritative header handling and duplicate/non-string form rejection protect the inventoried mutations. Authorization headers cannot bypass browser protection. Existing session/role checks remain. Multipart parsing precedes token validation but object-storage/business writes do not. Request-local token rendering, hidden fields and passkey fetch headers are wired; protected responses use `no-store`. - Legacy CLI route/template/registration are deleted; GET logout redirects without deleting sessions/cookies, protected POST deletes the valid session. The single-use magic-link callback exception is preserved. API/OpenAPI/federation contracts are not modified by the diff; bearer operations stay outside the browser guard. - No concrete newly introduced correctness defect identified in the inspected implementation. This does not establish live production topology, cookie behavior, identity alignment or protection. #### Acceptance criteria Statuses apply to the original rollout task, not a reduced implementation-only substitute. Local evidence is identified explicitly. 1. **met** — Explicit scoped backport, upstream commit and site-owned route/form customization recorded in `.slugkit-site.json` and `docs/csrf-rollout.md`. This describes repository adoption, not deployment adoption. 2. **partial** — Local deterministic tests cover the complete 16-route mutation inventory, invalid/missing/cross-origin/expired/wrong-binding submissions and no-write/no-upload assertions; local browser coverage exercises legitimate account/user/actor/setup/key/passkey/email/logout flows and an ambient-cookie sibling-Origin attack. Equivalent safe public verification is still pending. 3. **met** — `/cli/auth` is removed without redirect/shim; GET logout is non-mutating; current credential guidance uses `/settings/api-keys` and API-only CLI login. Supported by implementation and local regression evidence, not a claim about the currently deployed image. 4. **partial** — Origin precedence, forwarding-header rejection, cookies, session/context/key rotation and replica/restart behavior are implemented/documented and tested locally, with no HSTS adoption. Actual proxy/backend topology, stored production site URL/WebAuthn/email identity, replicas and operator-owned signing-key mechanism remain unverified/unapproved. 5. **met** — Current bearer API/CLI contract and federation implementation remain unchanged and browser-token-free in the diff; token-free bearer auth/write tests and existing local compatibility suite pass. No live federation testing or CLI release is claimed. 6. **met** — Existing logs show build/lint success, 437 local tests/50 files, 361 deterministic CI tests/43 files, 34 browser checks, and local proposed-version `0.3.12` image build/validation. `/tmp/evc-csrf-ci-evidence.txt` records successful Forgejo job #66 on the exact reviewed head. CI uses `test:ci`; release runtime smoke execution is removed; browser/federation/runtime testing stays local-only, with workflow-policy tests. Logs are historical evidence, not freshly rerun checks or public deployment verification. 7. **not-evident** — No explicit human production deployment approval, publication/deployment result or actual public positive/negative verification exists. Operator confirmed Docker Hub publication auto-deploys production; no deployment, runtime configuration or backup changes were approved/performed. Local checks cannot satisfy this criterion. 8. **partial** — Reviewed plan records provenance, local evidence, consistent SQLite/media backup approach, restore/pause/pin prerequisites, immutable-image rollback and safe synthetic public checks. Header task `task-dae6192b` remains active/unchanged; nonce/middleware coordination and separate HSTS decision are documented. Actual backup/restore readiness, auto-deployer rollback control and rollout evidence remain outstanding. Counts: **8 total; 4 met; 3 partial; 1 not-evident**. `criteria_missing=4` means all criteria not fully met (partial plus not-evident). #### Blocking issues (2) 1. **Operator prerequisite evidence missing (criteria 4/8):** Before publication, obtain sanitized evidence of canonical HTTPS/proxy/backend/Host behavior, stored site URL and identity alignment, replica/restart strategy and secret-management mechanism; identify the current immutable image and deployment configuration, recoverable consistent DB/media backups and verified restore/auto-deploy pause/pin rollback control. The plan correctly requires approval before any runtime/secret/backup change; proposed instructions are not proof of readiness. Never expose secret values or inspect unrelated personal records. 2. **Approved rollout and public verification absent (criteria 2/7/8):** Complete the explicit human merge gate and prerequisite review, then obtain separate explicit production deployment approval covering auto-deploy publication plus any approved configuration/secret/backup operations and synthetic verification scope. Only after that approval may deployment and safe actual public positive/negative checks occur. Record sanitized deployment/version/digest, workflow, verification and limitations, then rerun the review/closure gates. This review does not direct unauthorized deployment and does not close the task. #### Warnings (1) 1. Existing image-build installation output reports **8 runtime dependency vulnerabilities (6 moderate, 2 high)** and **17 total including development dependencies (8 moderate, 9 high)**. Runtime versions are unchanged by this backport, but the supplied logs do not establish advisory applicability or remediation. Treat this as a non-blocking implementation warning, not proof of deployment safety: triage and record any deployment-blocking advisory before seeking release approval; do not apply broad audit fixes implicitly. #### Deployment-plan disposition and final verdict - The plan correctly separates merge from publication/deployment approval, accounts for auto-deploy, avoids copying a live WAL database alone, requires controlled rollback, and warns that returning to `0.3.11` restores CSRF exposure. Safe synthetic verification and separate approval for destructive restore/rotation are appropriate. Operational evidence and approvals remain pending; the plan is not an executable production authorization yet. - **Final verdict: changes-requested** because the full task has a not-evident acceptance criterion and partial operational gates, despite substantial locally verified implementation work. Required next steps are the operator evidence and explicit approval sequence above, followed by authorized rollout/public evidence and renewed review. Do not relabel these criteria met from local logs, implicitly complete the header task, publish a CLI/image release, merge, or close this task. - Human merge decision remains pending; this review grants neither merge approval nor production approval.

erik added 1 commit 2026-10-05 13:45:54 -05:00

docs: record verified GitOps topology and canonical hostname blocker 96af908382

erik added 1 commit 2026-10-05 13:58:08 -05:00

docs: link reviewed homelab preparation to the CSRF rollout 93d61725c1

erik added 1 commit 2026-10-05 14:07:58 -05:00

docs: record fail-closed backup recipe verification bd95821d5b

erik merged commit c3d34b3f20 into main 2026-10-05 15:04:29 -05:00

erik deleted branch task-c6c678da-csrf 2026-10-05 15:04:29 -05:00

erik referenced this pull request from a commit 2026-10-05 15:04:30 -05:00

Merge pull request 'Backport browser CSRF protection and gate automatic production rollout' (#19) from task-c6c678da-csrf into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

bug

  

priority:high

  

status:active

  

status:inprogress

No labels

bug

priority:high

status:active

status:inprogress

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

erik/erikvancraddock.com!19

No description provided.