ohm: migrate orphaned subagent transcripts (0019.x, 0026.x) + renumber dup 0046.0 -> 0073
Backfills transcripts that existed only locally in ohm-infra: - 0019.1/.2/.3 - UX-polish wave subagent transcripts (driver 0019.0 never finalized) - 0026.1-.9 - security-audit-0026 subagent transcripts (driver abandoned/closed-out by 0068; audit drove published 0030 remediation) - 0073.0 - PPE/progressive-delivery + engineering-handbook session, originally drafted as a duplicate 0046.0; reassigned next free number (0072 taken by a concurrent session) sessions.json: add 0019/0073 titles, update 0026 title. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,200 @@
|
||||
# SESSION-0026.1 — AuthN/AuthZ security audit subsession (rfc-app v0.24.0)
|
||||
|
||||
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
Date: 2026-05-28T13-46 → 2026-05-28T13-50 (America/Los_Angeles)
|
||||
Role: subsession 0026.1 of OHM security-audit driver session 0026.
|
||||
Mode: READ-ONLY. No edits, no deploys, no mutations. Only file written is this transcript.
|
||||
|
||||
## Goal
|
||||
|
||||
Audit the authentication / authorization surface of the rfc-app codebase
|
||||
(`/Users/benstull/git/rfc-app`, tag v0.24.0): the email/OTC login flow,
|
||||
passcodes, device-trust, Gitea OAuth + core auth, session/cookie security,
|
||||
the admin role model, and per-endpoint authz guards across every
|
||||
`api*.py` write/admin/owner endpoint. Verify deployment guarantee #4
|
||||
("anonymous users are off-limits for any write/mutation"). Hunt for missing
|
||||
guards, IDOR, and OTC/passcode brute-force/replay.
|
||||
|
||||
## Outcome
|
||||
|
||||
One High finding (OTC verify has no attempt limit / lockout — asymmetric
|
||||
with the passcode path which does). Two Low/Info findings (session cookie
|
||||
`https_only=False`; email-bounce webhook unauthenticated when env unset —
|
||||
both documented dev contracts). Every enumerated write/admin/owner endpoint
|
||||
enforces auth; no missing guards, no IDOR found. Admin role model is sound.
|
||||
|
||||
## Pre-state (what I found at the start)
|
||||
|
||||
- `auth.py` defines the dependency ladder: `require_user` (401 if anon) →
|
||||
`require_contributor` (adds muted + permission_state='granted' gate) →
|
||||
`require_admin` (role in owner/admin). Plus per-RFC predicates
|
||||
(`can_contribute_to_rfc`, `can_discuss_rfc`, `can_invite_to_rfc`,
|
||||
`is_rfc_owner`). `current_user` re-reads role + permission_state from DB
|
||||
every request (so grants/revokes take effect next request — good).
|
||||
- Credentials: OTC 6-digit codes (bcrypt at rest, 10-min TTL, 60s request
|
||||
cooldown, single live code per email); passcodes (bcrypt, 5-fail/15-min
|
||||
per-account lockout, denylist, 4–20 chars); device-trust (256-bit CSPRNG,
|
||||
bcrypt, 30-day, HttpOnly+Secure+SameSite=Lax cookie); invite tokens
|
||||
(256-bit CSPRNG, bcrypt, 7-day, single-use).
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Read auth.py + config.py. Dependency ladder is clean. Noted SECRET_KEY
|
||||
is `_required` (good — no insecure default). OAuth provisioning sets
|
||||
owner role only via OWNER_GITEA_LOGIN match; OTC/invite users get
|
||||
gitea_login="" / gitea_id=0 coalesced.
|
||||
|
||||
2. Read otc.py + passcode.py side by side. **Caught the asymmetry**:
|
||||
`passcode.verify_passcode` has a per-account failed-attempts counter +
|
||||
15-min lockout; `otc.verify_code` has NOTHING — it walks the last 5
|
||||
rows, bcrypt-checks, and returns "wrong" with no counter, no lockout,
|
||||
no per-IP throttle. The 60s cooldown lives in `request_code`, not
|
||||
`verify_code`. This is the headline finding.
|
||||
|
||||
3. Read device_trust.py + email_otc.py. Device-trust is solid (constant-time
|
||||
bcrypt walk, no early-exit, token never logged, cookie flags correct,
|
||||
user-scoped revoke). email_otc always returns 202 / logs failures —
|
||||
no email enumeration via the request path.
|
||||
|
||||
4. Read main.py (middleware + all /auth endpoints). Confirmed:
|
||||
- SessionMiddleware uses `https_only=False` (line 150) — itsdangerous-
|
||||
signed cookie, HttpOnly by default, but Secure flag off. Dev parity
|
||||
comment present. Behind HTTPS+proxy in prod this is low-impact but
|
||||
worth flagging.
|
||||
- `/auth/otc/verify` has NO rate-limit dependency, confirming finding #1
|
||||
at the endpoint layer too.
|
||||
- Turnstile gates `/auth/otc/request` ONLY, not verify.
|
||||
|
||||
5. Refutation pass on finding #1: Is there a global limiter? Grepped for
|
||||
RateLimit/slowapi/limiter/429 across backend/app — only tag_suggest.py
|
||||
(per-user, in-memory) and the OTC *request* cooldown 429. No global
|
||||
middleware, no limiter on verify. SPEC §ahead-of (line ~4100) discusses
|
||||
per-IP throttle for the *passcode* verify (which already has a per-
|
||||
account lockout) but never notes the OTC verify has no per-account limit
|
||||
at all. Quantified: single live code per email, bcrypt ~tens of ms,
|
||||
10^6 keyspace, 10-min TTL. Full sweep not possible in one TTL, but
|
||||
attacker re-requests (60s cooldown) to keep a live code and runs
|
||||
parallel verifies; expected ~500k guesses. bcrypt cost + TTL + single-
|
||||
live-code = real friction but NO attempt cap. Downgraded Critical→High.
|
||||
|
||||
6. Read api_admin.py end to end. Every endpoint calls require_admin; the
|
||||
user-search typeahead intentionally uses require_user (no privileged
|
||||
data). Role escalation well-guarded: only owner grants/changes owner;
|
||||
self-downgrade, self-mute, self-permission-flip, self-invite all refused;
|
||||
admins/owners not write-mutable. No spoof/escalation path. SOLID.
|
||||
|
||||
7. Enumerated routes+guards across api.py, api_prs.py, api_branches.py,
|
||||
api_graduation.py, api_invitations.py, api_notifications.py,
|
||||
api_discussion.py via grep. Every write/mutation route has require_user
|
||||
/ require_contributor / require_admin, and the per-RFC write routes
|
||||
additionally call can_contribute_to_rfc / can_discuss_rfc /
|
||||
can_invite_to_rfc. GET read routes use current_user (nullable) — anon
|
||||
read is by design (guarantee #4 is about writes).
|
||||
|
||||
8. IDOR hunt — checked every owner/id-comparing helper:
|
||||
- api_prs `_can_merge`/`_can_withdraw`/`_can_edit_pr_text`: compare
|
||||
viewer.gitea_login against RFC frontmatter owners/arbiters cache and
|
||||
pr_row["opened_by"] — server-side data, not user-supplied. Safe.
|
||||
- api_branches `_require_branch_owner`/`_can_contribute`/
|
||||
`_can_read_branch`: branch grants keyed on grantee_user_id (session
|
||||
id), creator compared to gitea_login. Safe.
|
||||
- notification mute (`/api/users/{user_id}/notification-mute`): SQL
|
||||
scoped to viewer.user_id; only the {user_id} *target* is user-supplied
|
||||
and it's just the mute subject, not an ownership pivot. Self-mute
|
||||
refused. Safe.
|
||||
- invitations accept: requires auth + case-insensitive email match +
|
||||
refuses empty email; collaborator row keyed on viewer.user_id. Safe.
|
||||
- Considered: OTC/invite users have gitea_login="". Could "" match an
|
||||
owners list? Only if frontmatter literally contained "" — impossible
|
||||
from gitea_login frontmatter. `"" in []` is False. Killed as non-issue.
|
||||
|
||||
9. Checked anonymous-reachable mutation surfaces:
|
||||
- `/api/email/unsubscribe` (GET+POST): itsdangerous URLSafeSerializer
|
||||
signed token (SECRET_KEY, salted) → user_id+category not forgeable;
|
||||
idempotent, low-stakes; no expiry but URLSafeSerializer is fine here.
|
||||
- `/api/webhooks/email-bounce`: optional X-Webhook-Secret (constant-time
|
||||
compare) when WEBHOOK_EMAIL_BOUNCE_SECRET set; UNAUTHENTICATED when
|
||||
unset (documented v1 dev contract). Impact: flip email opt-out flags +
|
||||
mark outbound rows bounced. Low.
|
||||
- `/api/webhooks/...` Gitea receiver: mandatory HMAC-SHA256, constant-
|
||||
time compare, refuses to start without secret unless
|
||||
RFC_APP_INSECURE_WEBHOOKS=1. SOLID.
|
||||
- `/api/invites/claim`, `/auth/device-trust/start`: anon by design (they
|
||||
establish the session); both gated by 256-bit bcrypt-hashed tokens.
|
||||
|
||||
10. Confirmed db is a single shared sqlite connection (check_same_thread=
|
||||
False, isolation_level=None) — no incidental concurrency limiter that
|
||||
would mitigate the OTC brute-force.
|
||||
|
||||
## Cut state (final findings)
|
||||
|
||||
### High
|
||||
- **F1 — OTC verify has no attempt limit / lockout / per-IP throttle.**
|
||||
`backend/app/otc.py:211 verify_code` (endpoint `backend/app/main.py:279
|
||||
/auth/otc/verify`). The passcode path (`passcode.py:262`) has a 5-fail/
|
||||
15-min per-account lockout; the OTC path has none. 60s cooldown is on
|
||||
*request* only. 6-digit code (10^6), 10-min TTL, single live code per
|
||||
email. Remediation: add a per-account failed-OTC-attempt counter +
|
||||
lockout mirroring passcode.py, and/or a per-IP 429 throttle on verify;
|
||||
consider invalidating the live code after N failures. Confidence: HIGH
|
||||
the control is absent (verified across module + endpoint + global grep).
|
||||
Severity argued down from Critical to High because bcrypt cost + 10-min
|
||||
TTL + single-live-code give real friction; still a genuine gap and an
|
||||
inconsistency with the passcode path.
|
||||
|
||||
### Low / Info
|
||||
- **F2 — Session cookie `https_only=False`.** `backend/app/main.py:150`
|
||||
SessionMiddleware. Cookie is itsdangerous-signed + HttpOnly, but Secure
|
||||
is off "for dev parity." The device-trust cookie correctly sets
|
||||
Secure=True. In prod behind HTTPS the session cookie should also be
|
||||
Secure. Remediation: drive https_only from an env flag, true in prod.
|
||||
Confidence: HIGH it's set false; impact Low given HTTPS + signed cookie.
|
||||
- **F3 — Email-bounce webhook unauthenticated when env unset.**
|
||||
`backend/app/api_notifications.py:543`. Documented v1 dev contract;
|
||||
impact limited to email opt-out flag flips + outbound-row status. Info/
|
||||
Low. Confidence HIGH on behavior, low on impact.
|
||||
|
||||
### What's solid (controls correctly implemented)
|
||||
- Three-tier dependency ladder; `current_user` re-reads role +
|
||||
permission_state from DB every request (revokes take effect immediately).
|
||||
- Admin role model: owner-only owner grants, self-downgrade/self-mute/
|
||||
self-permission/self-invite refusals, admins/owners not write-mutable.
|
||||
No escalation/spoof path. Every /api/admin/* endpoint require_admin.
|
||||
- Every write/mutation endpoint across all api*.py files enforces auth;
|
||||
per-RFC writes additionally enforce can_*_to_rfc. No missing guard.
|
||||
- No IDOR: owner/permission checks compare against server-side frontmatter
|
||||
cache / session user_id / pr opened_by, never a trusted user-supplied id.
|
||||
- Passcode lockout (5/15min), denylist; OTC single-live-code + consume-
|
||||
before-provision (no replay, no double-sign-in).
|
||||
- All tokens (OTC, passcode, device-trust, invite) bcrypt-hashed at rest;
|
||||
device-trust/invite tokens 256-bit CSPRNG; constant-time bcrypt walks,
|
||||
no early-exit timing leak; raw tokens never logged.
|
||||
- Gitea webhook mandatory HMAC-SHA256 + constant-time compare; OAuth state
|
||||
param checked with secrets.compare_digest.
|
||||
- SECRET_KEY is _required (no insecure default); unsubscribe tokens signed
|
||||
with it (salted, not forgeable).
|
||||
- OTC request returns 202 regardless (no email enumeration); verify
|
||||
collapses wrong/unknown to one 400 (no enumeration); passcode verify
|
||||
collapses wrong/unknown likewise.
|
||||
|
||||
## What the driver needs to know
|
||||
|
||||
- F1 is the only finding worth a remediation slice. It is a real,
|
||||
exploitable-with-effort gap AND an internal inconsistency (passcode has a
|
||||
lockout, OTC doesn't). Recommend the driver fold it in at High; if the
|
||||
driver judges the bcrypt+TTL friction insufficient given OHM is publicly
|
||||
reachable, it could argue back toward Critical — I left it at High after
|
||||
refutation.
|
||||
- F2/F3 are documented dev contracts; flag them but they are low priority.
|
||||
Worth a one-line confirmation that OHM's prod overlay sets
|
||||
WEBHOOK_EMAIL_BOUNCE_SECRET and serves over HTTPS (the device-trust
|
||||
cookie is Secure-only, so if OHM weren't HTTPS, device trust would
|
||||
already be broken — implying HTTPS is in fact present, which de-risks F2).
|
||||
- The authz guard coverage is genuinely complete — guarantee #4 holds for
|
||||
every write/admin/owner endpoint I enumerated. No anon-write hole found.
|
||||
|
||||
## §19.2 candidates surfaced
|
||||
|
||||
- Per-account OTC-verify lockout + per-IP verify throttle (pairs with the
|
||||
SPEC's already-listed per-IP passcode-verify candidate). This is the
|
||||
remediation for F1 and is a flotilla-irrelevant, rfc-app-core change.
|
||||
@@ -0,0 +1,136 @@
|
||||
# SESSION-0026.2 — rfc-app Injection & Input-Handling Security Audit
|
||||
|
||||
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
Subsession: 0026.2 (read-only audit leg of security-audit driver session 0026)
|
||||
Date: 2026-05-28 (PST)
|
||||
Target: rfc-app at /Users/benstull/git/rfc-app, tag v0.24.0 (HEAD 28015ed)
|
||||
Scope: SQL injection, XSS, SSRF, path traversal, email/markdown sanitization.
|
||||
Mode: READ-ONLY. No edits, no deploys, no git mutations. No secret bytes recorded.
|
||||
|
||||
## Pre-state
|
||||
|
||||
Driver assigned the injection & input-handling surface. Repo confirmed at
|
||||
tag v0.24.0. Backend is FastAPI + SQLite (backend/app/*.py); frontend is
|
||||
React 19 + Vite (frontend/src). Markdown via `marked` v18; no sanitizer
|
||||
library in frontend/package.json dependencies.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Enumerated backend modules and grepped all 321 `execute*` call sites for
|
||||
f-string / `.format` / `%` / concat SQL. Pulled the ~16 dynamic-SQL hits.
|
||||
|
||||
2. SQL injection — examined every dynamic-SQL site:
|
||||
- api_notifications.py:274 (`UPDATE users SET {', '.join(sets)}`) — `sets`
|
||||
contains only hardcoded column-name literals; values parameterized via
|
||||
`args`. Safe.
|
||||
- api_notifications.py:479/483 (`UPDATE users SET {column} = ...`) —
|
||||
`column` from static `_CATEGORY_COLUMN` allowlist. Safe.
|
||||
- api_admin.py:413, notify.py:535, digest.py:201, email.py:610,
|
||||
notify.py:942/964/1000 — all IN-clauses built via `",".join("?" * len)`,
|
||||
the correct parameterized-IN idiom; values in the tuple. Safe.
|
||||
- notify.py:638/643 — `{now}` is the literal `datetime('now')`, not input.
|
||||
- api_admin.py:629/711/754 — `WHERE` built from `clauses` lists holding
|
||||
only fixed `col = ?` strings; user values parameterized; `limit` is
|
||||
FastAPI `int` with ge/le bounds. Safe.
|
||||
- db.py — connection/migration layer; migrations read trusted on-disk .sql.
|
||||
- Swept for dynamic ORDER BY / LIMIT / table-name injection: none found.
|
||||
Verdict: parameterization is consistent and correct across the backend.
|
||||
|
||||
3. XSS — found the markdown pipeline:
|
||||
- frontend/package.json has NO DOMPurify / rehype-sanitize / any sanitizer.
|
||||
- MarkdownPreview.jsx:101-102 — `previewMarked.parse(content)` →
|
||||
`hostRef.current.innerHTML = html`. The custom `Marked` instance only
|
||||
overrides the ```mermaid code-fence renderer; everything else is marked
|
||||
default (raw HTML passthrough). `escapeHtml` is applied ONLY to the
|
||||
mermaid placeholder text and error text, NOT the document body.
|
||||
- ProposalView.jsx:164 & 172 — `dangerouslySetInnerHTML` with the GLOBAL
|
||||
unconfigured `marked.parse(data.entry?.body)` / `proposed_use_case`.
|
||||
- Content sources: RFCView passes RFC body (previewContent/editorContent)
|
||||
into MarkdownPreview; DocsSessionTranscript / DocsSpec / DocsUserGuide /
|
||||
Philosophy / DocsSessionsAbout/Index also feed body through it. RFC body
|
||||
and proposal body/use-case are free-text authored by any invited user.
|
||||
- Backend: no sanitization on write (api.py — only Field max_length caps).
|
||||
- Self-refutation step: ran `marked.parse` on
|
||||
`<img src=x onerror=...> <script> [xss](javascript:...)`. Output passes
|
||||
all three through verbatim. With innerHTML/dangerouslySetInnerHTML the
|
||||
`<img onerror>`, `<svg onload>`, event handlers and `javascript:` links
|
||||
execute (the bare `<script>` won't via innerHTML, but the rest do).
|
||||
Confirmed stored XSS. Killed nothing — sink reachable, input
|
||||
attacker-controlled, no upstream sanitizer.
|
||||
- Editor.jsx:125 feeds marked output into TipTap `setContent`. NOT a raw
|
||||
sink — ProseMirror's schema normalizes/drops unknown nodes & attrs.
|
||||
Marked this path as solid, not a finding.
|
||||
|
||||
4. SSRF — checked every outbound-fetch module:
|
||||
- gitea.py: httpx client `base_url` fixed from config; owner/repo/path are
|
||||
gitea API path segments, not arbitrary hosts.
|
||||
- docs_sessions.py / docs_specs.py: fetch from raw/contents bases that come
|
||||
from env (operator) or framework defaults; the per-spec `url` is taken
|
||||
only from the operator-configured OHM_DOCS_SPECS manifest, never from
|
||||
user input — user can only select a pre-registered slug.
|
||||
- providers.py: provider base_url/model from env only.
|
||||
- turnstile.py: siteverify URL from env or fixed constant.
|
||||
- email.py: SMTP host from env.
|
||||
No user-controlled fetch URL anywhere. No SSRF.
|
||||
|
||||
5. Path traversal — docs_sessions / docs_specs:
|
||||
- api.py:242-280 validates `nnnn` against `^\d{4}$` and `filename` against
|
||||
a fully-anchored `_TRANSCRIPT_FILENAME_RE` (only SESSION-...md shape, no
|
||||
`/`/`..`) BEFORE any network call. docs_specs uses anchored
|
||||
`^[a-z0-9-]+$` slug guard, re-checked inside fetch_spec as defense in
|
||||
depth. FastAPI `{filename}` path param won't match `/` by default; even a
|
||||
decoded `%2f` fails the anchored regex. No traversal.
|
||||
|
||||
6. Email / markdown sanitization on backend:
|
||||
- email_invite.py / email_otc.py / email.py / digest.py all call
|
||||
build_envelope with body_plain only. Grep confirms NO caller passes
|
||||
`body_html=` anywhere — the multipart/alternative HTML branch in
|
||||
email_envelope.py:134-140 is dead in practice. All mail is plain text;
|
||||
user content (custom_message, display names) is plain-text interpolated.
|
||||
No email HTML-injection vector at v0.24.0.
|
||||
|
||||
## Cut state — findings
|
||||
|
||||
- F1 [High] Stored XSS via unsanitized markdown→HTML. marked + raw HTML sink
|
||||
(innerHTML / dangerouslySetInnerHTML), no sanitizer in pipeline or deps.
|
||||
Sinks: MarkdownPreview.jsx:101-102; ProposalView.jsx:164,172. Sources:
|
||||
RFC body, proposal body/use-case (any invited user), session transcripts.
|
||||
Confidence: HIGH (empirically confirmed marked passthrough; sink reachable;
|
||||
no upstream sanitization; backend stores raw). Invite-gating narrows the
|
||||
attacker pool but stored XSS by an invited user against admins/others
|
||||
(incl. admin session/cookie theft) remains. Fix: add DOMPurify and sanitize
|
||||
marked output before every innerHTML/dangerouslySetInnerHTML write (single
|
||||
shared helper), or render via a sanitizing markdown component; disable raw
|
||||
HTML and screen javascript:/data: URIs.
|
||||
|
||||
- F2 [Info] Dead HTML-email branch. email_envelope.py supports a text/html
|
||||
alternative but no caller supplies body_html. Not exploitable today; if
|
||||
HTML email is ever added, user content must be HTML-escaped first. Track so
|
||||
it's not turned on without escaping.
|
||||
|
||||
## What's solid
|
||||
|
||||
- SQL: consistent, correct parameterization across all ~16 dynamic-SQL sites;
|
||||
IN-clauses use the `"?" * len` idiom; dynamic column/where fragments are
|
||||
hardcoded allowlists; no ORDER BY/LIMIT/table-name injection; limits are
|
||||
bounded ints.
|
||||
- SSRF: zero user-controlled outbound-fetch URLs; all bases env/config-fixed.
|
||||
- Path traversal: anchored-regex validation at the route layer before any
|
||||
network call on both docs surfaces; defense-in-depth re-check in docs_specs.
|
||||
- Editor.jsx markdown flows through TipTap/ProseMirror schema (not a raw sink).
|
||||
- Email is plain-text only today.
|
||||
|
||||
## What the driver needs to know
|
||||
|
||||
- One real, shippable finding: F1 stored XSS. It is the highest-impact item
|
||||
on this surface. Recommend a single sanitize helper (DOMPurify) wrapping
|
||||
every marked render; covers MarkdownPreview, ProposalView, and all docs
|
||||
views at once.
|
||||
- Everything else on this surface is clean. SQL/SSRF/path-traversal are well
|
||||
built and consistent — worth calling out as defensive strengths in the
|
||||
combined audit report.
|
||||
|
||||
## §19.2 candidates (flotilla-core extraction)
|
||||
|
||||
- None from this audit — findings are rfc-app application-layer, not flotilla
|
||||
CLI / deployment-shape concerns.
|
||||
@@ -0,0 +1,81 @@
|
||||
# SESSION-0026.3 — OHM security audit subsession: webhooks, CAPTCHA, email
|
||||
|
||||
Parent: [SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md](./SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md)
|
||||
Subsession: 0026.3
|
||||
Mode: READ-ONLY security audit. No mutations. Only file written: this transcript.
|
||||
Target: rfc-app @ v0.24.0 (commit 28015ed), /Users/benstull/git/rfc-app
|
||||
Date: 2026-05-28, America/Los_Angeles
|
||||
|
||||
## Surface
|
||||
Webhooks (backend/app/webhooks.py), Turnstile/CAPTCHA (backend/app/turnstile.py),
|
||||
email (email.py, email_otc.py, email_invite.py, email_envelope.py + the
|
||||
api_invitations.py / api_notifications.py call/receive sites).
|
||||
|
||||
## Pre-state
|
||||
Came in with the three-part brief (webhook HMAC/replay/unknown-repo; Turnstile
|
||||
verify/fail-open/single-use/coverage; email header-injection/redirect/SSRF).
|
||||
No prior knowledge of these modules.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Confirmed tag v0.24.0, read webhooks.py + turnstile.py in full.
|
||||
- Webhook: HMAC-SHA256 over the *raw* `await request.body()` bytes (good),
|
||||
`hmac.compare_digest` (good), secret required at config-load (config.py
|
||||
refuses to start without GITEA_WEBHOOK_SECRET unless RFC_APP_INSECURE_WEBHOOKS=1).
|
||||
Verification happens BEFORE any json.loads / cache action. Unknown-repo path
|
||||
(#18) only logs — no unsafe action. No replay/dedup (delivery-id/timestamp).
|
||||
- Turnstile: success field checked; network/parse failure -> reason="network"
|
||||
(fail-CLOSED only when TURNSTILE_REQUIRED=true; fail-OPEN by default when
|
||||
secret unset). Token never cached -> single-use enforced by Cloudflare.
|
||||
|
||||
2. Read all email modules + envelope builder. Found a 5th sender (api_invitations.py
|
||||
RFC-invite) beyond the 4 named in the brief — also routes through build_envelope.
|
||||
|
||||
3. Located enforcement sites: Turnstile wired ONLY to /auth/otc/request (main.py:255).
|
||||
Checked beta-request (api.py:441, require_user) and propose (api.py:766,
|
||||
require_contributor) — both require an authenticated session, so Turnstile is
|
||||
correctly unneeded there. The unauthenticated abuse hot path (OTC request) is
|
||||
the one that needs it, and it's covered. -> KILLED the "Turnstile missing on
|
||||
beta/propose" candidate.
|
||||
|
||||
4. Header-injection probe. Empirically tested Python 3.13 EmailMessage:
|
||||
- `m["To"]=`, `m["Subject"]=`, and `formataddr((display,addr))` assignment all
|
||||
raise ValueError on embedded CR/LF (default EmailPolicy). Code uses
|
||||
EmailMessage (NOT legacy email.mime), so the classic header-injection vector
|
||||
is closed at the stdlib layer for To/From/Subject/display-name.
|
||||
-> KILLED "CRLF header injection / BCC smuggling" as exploitable. Downgraded to
|
||||
a minor availability note (a malicious display name -> uncaught ValueError ->
|
||||
500), needs-verification on whether that path is reachable with attacker text.
|
||||
|
||||
5. email-bounce webhook (api_notifications.py:543): unauthenticated when
|
||||
WEBHOOK_EMAIL_BOUNCE_SECRET unset (the documented v1 dev default); flips
|
||||
email_opt_out_all=1 for any matching user email. Constant-time compare when set.
|
||||
|
||||
6. Confirmed unsubscribe tokens: itsdangerous URLSafeSerializer signed with
|
||||
SECRET_KEY, salt "email-unsubscribe", scoped to (user_id, category). No expiry
|
||||
(intentional, revocable by rotating SECRET_KEY). app_url is operator-controlled
|
||||
(no open-redirect via user input). siteverify URL is fixed (no SSRF).
|
||||
|
||||
## Cut state — findings
|
||||
|
||||
- F1 (LOW/Medium-conf): email-bounce webhook unauthenticated by default
|
||||
(WEBHOOK_EMAIL_BOUNCE_SECRET unset) -> unauth attacker can force global
|
||||
email opt-out for any known address (DoS on a user's mail). Mitigated in prod
|
||||
IF the secret is wired; needs-verification that OHM's overlay sets it.
|
||||
- F2 (INFO): no webhook replay protection (no delivery-id/timestamp dedup) on
|
||||
the Gitea receiver. Low impact — handlers are idempotent cache refreshes, no
|
||||
state mutation from payload contents. Acceptable; documented for completeness.
|
||||
- F3 (LOW/availability, needs-verification): malicious display-name containing
|
||||
CR/LF reaching build_envelope raises uncaught ValueError -> 500. Not injection.
|
||||
- KILLED: CRLF header injection / BCC smuggling (stdlib guard).
|
||||
- KILLED: Turnstile missing on beta-request/propose (both auth-gated).
|
||||
- KILLED: Turnstile token replay (not cached; Cloudflare enforces single-use).
|
||||
|
||||
## Driver needs to know
|
||||
- Verify OHM overlay sets WEBHOOK_EMAIL_BOUNCE_SECRET (F1) and
|
||||
TURNSTILE_REQUIRED=true (else Turnstile fails OPEN if the secret regresses).
|
||||
- These are env/overlay checks for the flotilla side, not rfc-app code bugs.
|
||||
|
||||
## §19.2 candidates
|
||||
- Webhook delivery-id dedup could become a shared flotilla-core concern if more
|
||||
webhook receivers land. Not urgent.
|
||||
@@ -0,0 +1,172 @@
|
||||
# SESSION-0026.4 — OHM security-audit subsession (newest endpoints, rate-limiting/abuse, frontend posture)
|
||||
|
||||
- **Role:** subsession 0026.4 of OHM security-audit driver session 0026.
|
||||
- **Parent transcript:** SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
- **Mode:** READ-ONLY. No mutations. No secret bytes printed.
|
||||
- **Target:** rfc-app @ /Users/benstull/git/rfc-app, tag v0.24.0 (HEAD 28015ed).
|
||||
- **Window:** 2026-05-28T13-46 → 13-50 (America/Los_Angeles).
|
||||
|
||||
## Surface assigned
|
||||
Newest endpoints from session 0022 work (proposed_use_cases writes #26,
|
||||
PUT /api/me/last-state #29, Claude Haiku tag suggestions #27); rate
|
||||
limiting / abuse on OTC + beta-request + suggest-tags + propose; frontend
|
||||
token/secret handling, VITE_ public-vs-secret discipline, token storage,
|
||||
CSP/security headers, npm audit.
|
||||
|
||||
## Pre-state
|
||||
Fresh read-only checkout at v0.24.0. No prior findings in this subsession.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Enumerated backend/app + frontend/src layout, confirmed v0.24.0 tag.
|
||||
2. Read tag_suggest.py in full, useLastState.js, main.py (full).
|
||||
- tag_suggest: output strictly constrained to the corpus tag universe
|
||||
(parse_reply maps replies back to canonical spellings, drops invented
|
||||
tags). Prompt-injection into the Haiku call can't escape the tag set;
|
||||
worst case is a junk/empty suggestion list. Per-user in-process rate
|
||||
limit (TAG_SUGGEST_RATE_MAX default 30/60s) + require_contributor gate
|
||||
+ draft truncation (title 300, pitch/use_case 4000) + universe cap 200
|
||||
bound the LLM cost surface. SOLID.
|
||||
3. Located + read the #26 use_case writes (api.py propose, api_prs.py
|
||||
open-pr), the #29 last-state route, the suggest-tags route.
|
||||
- last-state: keyed on user.user_id from the session in BOTH read
|
||||
(/api/auth/me) and write (PUT /api/me/last-state). No user-supplied
|
||||
id anywhere → no cross-user read/write (IDOR). require_user gate.
|
||||
resume_enabled opt-out respected. route bounded 2048, state is a
|
||||
dict serialized as JSON. SOLID on authz.
|
||||
4. Checked propose + open-pr authz: both require_contributor (granted,
|
||||
non-muted, authenticated). use_case bounded max_length=8000. SOLID.
|
||||
5. Frontend rendering of proposed_use_case: grep found
|
||||
ProposalView.jsx:172 renders it via marked.parse() into
|
||||
dangerouslySetInnerHTML. No DOMPurify anywhere in the project
|
||||
(confirmed by grep across frontend/src + package.json).
|
||||
6. KILLED a refutation attempt: verified marked default does NOT sanitize.
|
||||
Ran node one-liner: marked.parse('<img src=x onerror=alert(1)>') emits
|
||||
the raw <img onerror> tag, and [a](javascript:alert(2)) emits a live
|
||||
javascript: href. So this is a real stored-XSS sink, not theoretical.
|
||||
7. Confirmed the XSS audience: GET /api/proposals/{pr_number} is
|
||||
ANONYMOUS-readable (no require_ gate; uses current_user only for
|
||||
affordances). Admins reviewing a proposal (who hold merge/decline)
|
||||
render the payload → privileged-victim XSS.
|
||||
8. Frontend token storage: grep for localStorage/sessionStorage/
|
||||
document.cookie/Authorization/Bearer in api.js → ZERO hits. Auth is
|
||||
server cookie-session (credentials:'include'); device-trust + session
|
||||
cookies are HttpOnly+Secure+SameSite=Lax (main.py). No token in JS
|
||||
reach → XSS can't exfil an auth token directly. SOLID.
|
||||
9. VITE_ discipline: all VITE_ vars are genuinely public — APP_NAME,
|
||||
BETA_CONTACT, TURNSTILE_SITE_KEY (public by design), AMPLITUDE_API_KEY
|
||||
(Amplitude browser ingestion key, public by design; .env.example
|
||||
explicitly documents it as `flotilla overlay set`, NOT secret), policy
|
||||
URLs. No secret shipped as a VITE_ var. SOLID.
|
||||
10. npm audit (frontend): 0 vulnerabilities (info/low/moderate/high/
|
||||
critical all 0). SOLID.
|
||||
11. OTC abuse: request_code has a per-email cooldown
|
||||
(OTC_REQUEST_COOLDOWN_SECONDS default 60) AND the route runs
|
||||
turnstile.verify_token BEFORE request_code (so a failed challenge
|
||||
spends no budget / sends no mail). Email-bomb + LLM-cost surfaces
|
||||
mitigated. Cooldown is per-email not per-IP, but Turnstile covers the
|
||||
bot/IP dimension. SOLID with one caveat (below).
|
||||
12. beta-request: require_user gated (not anon), admin notification fires
|
||||
once via the was_already_complete guard. Not anon-abusable.
|
||||
13. Security headers: app has only SessionMiddleware — no CSP / X-Frame /
|
||||
X-Content-Type / HSTS. nginx conf (deploy/nginx/ohm.wiggleverse.org.conf)
|
||||
sets only Cache-Control — NO security headers either. So nothing
|
||||
backstops the XSS sinks at the header layer.
|
||||
|
||||
## Cut state — findings
|
||||
|
||||
### F1 — Stored XSS via proposed_use_case rendered through marked + dangerouslySetInnerHTML (no sanitizer)
|
||||
- **Location:** frontend/src/components/ProposalView.jsx:172 (sink);
|
||||
source written at backend/app/api.py:841-854 (#26 propose) and
|
||||
backend/app/api_prs.py:186-199 (#26 open-pr); served unsanitized via
|
||||
GET /api/proposals/{pr_number} (api.py:716, anon-readable);
|
||||
proposed_use_case max_length=8000.
|
||||
- **Issue:** marked does not sanitize by default (verified live: raw
|
||||
`<img onerror>` and `javascript:` hrefs pass through), and the project
|
||||
has NO DOMPurify. A granted contributor's use_case text renders as
|
||||
live HTML in any viewer's browser, including admins who hold
|
||||
merge/decline on that proposal.
|
||||
- **Exploit:** contributor proposes an RFC with
|
||||
`<img src=x onerror=...>` in the use case; an admin opens the proposal
|
||||
to review it; script runs in the admin's authenticated session
|
||||
(CSRF-style writes via the admin's cookie, UI defacement). Auth-token
|
||||
exfil is blocked (HttpOnly cookies), but cookie-authenticated state-
|
||||
changing requests from the victim's browser are not.
|
||||
- **Remediation:** sanitize all marked output with DOMPurify before
|
||||
dangerouslySetInnerHTML (or configure a safe renderer), at minimum on
|
||||
the new proposed_use_case sink. Same pattern should cover the
|
||||
pre-existing entry-body sink (F2).
|
||||
- **Severity:** High. **Confidence:** High (live-verified marked
|
||||
passthrough; confirmed no sanitizer; confirmed privileged victim).
|
||||
|
||||
### F2 — Pre-existing same-sink XSS on entry body (in-scope by adjacency)
|
||||
- **Location:** ProposalView.jsx:164 (also Editor.jsx:125 preview path);
|
||||
marked.parse(data.entry?.body) → dangerouslySetInnerHTML.
|
||||
- **Issue:** identical unsanitized sink; the proposed entry body is
|
||||
proposer-controlled markdown. Predates v0.24.0 but shares the root
|
||||
cause and remediation with F1.
|
||||
- **Severity:** High. **Confidence:** High. (Flagged as the root-cause
|
||||
class so a fix covers both, not just the #26 field.)
|
||||
|
||||
### F3 — No security headers at app or nginx layer (defense-in-depth gap)
|
||||
- **Location:** backend/app/main.py (only SessionMiddleware);
|
||||
deploy/nginx/ohm.wiggleverse.org.conf (only Cache-Control).
|
||||
- **Issue:** no Content-Security-Policy, X-Frame-Options/frame-ancestors,
|
||||
X-Content-Type-Options, or HSTS. A CSP would meaningfully blunt F1/F2
|
||||
(block inline/onerror script execution). Absence means the XSS sinks
|
||||
have no header-layer backstop, and the app is clickjackable.
|
||||
- **Remediation:** add CSP (script-src self, no unsafe-inline),
|
||||
X-Frame-Options DENY / frame-ancestors none, X-Content-Type-Options
|
||||
nosniff, HSTS — at the nginx layer is simplest.
|
||||
- **Severity:** Medium. **Confidence:** High.
|
||||
|
||||
### F4 (minor / needs-verification) — OTC cooldown is per-email, not per-IP
|
||||
- **Location:** backend/app/otc.py:152-164.
|
||||
- **Issue:** the cooldown keys on email, so one attacker can rotate
|
||||
through many target emails (mail-bomb across recipients) without
|
||||
tripping a single email's cooldown. Mitigated by the mandatory
|
||||
Turnstile gate on /auth/otc/request (main.py:255) when TURNSTILE is
|
||||
wired; if TURNSTILE_REQUIRED=false and no secret is set (the default
|
||||
dev posture), the gate opens and only the per-email cooldown remains.
|
||||
- **Remediation:** keep Turnstile required in production; consider an
|
||||
IP-scoped request ceiling as belt-and-suspenders.
|
||||
- **Severity:** Low (prod-config-dependent). **Confidence:** Medium —
|
||||
hinges on the deployment's TURNSTILE_REQUIRED + secret state, which
|
||||
this subsession did not inspect (operator/§13 record, not code).
|
||||
|
||||
## What's solid
|
||||
- last-state #29: no IDOR (server-session user_id only), no open-redirect
|
||||
(resume only navigates client-side react-router routes from "/", with a
|
||||
NON_RESUMABLE_PREFIXES skip list; no external URL navigation), state
|
||||
blob is bounded + JSON-parsed defensively. Resume redirect is
|
||||
one-shot. SOLID.
|
||||
- tag_suggest #27: prompt-injection contained (output constrained to
|
||||
canonical tag universe), cost bounded (per-user rate limit + input
|
||||
truncation + universe cap + require_contributor gate), degrades to
|
||||
silence on any failure. SOLID.
|
||||
- Frontend secrets: no auth token in JS-reachable storage; cookies are
|
||||
HttpOnly+Secure+SameSite=Lax; no secret shipped as a VITE_ var
|
||||
(AMPLITUDE/TURNSTILE keys are public by design). SOLID.
|
||||
- npm audit: 0 advisories. SOLID.
|
||||
- OTC/beta-request authz + cost: Turnstile-gated OTC with per-email
|
||||
cooldown; beta-request require_user + once-only admin notify. SOLID
|
||||
(modulo F4 caveat).
|
||||
|
||||
## Driver-needs-to-know
|
||||
- F1 is the headline: a real, live-verified stored XSS in the #26
|
||||
use-case field that can land on an admin reviewer. F2 is the same sink
|
||||
on the entry body — fix the sanitizer once, cover both. F3 (no CSP)
|
||||
removes the only header-layer backstop. Recommend a single remediation
|
||||
wave: add DOMPurify around every marked→dangerouslySetInnerHTML sink +
|
||||
add a CSP at nginx.
|
||||
- F4 depends on the OHM deployment's TURNSTILE_REQUIRED + secret state
|
||||
(a §13 record / operator question, not in this code), so verify that
|
||||
before downgrading or upgrading it.
|
||||
|
||||
## §19.2 candidates (flotilla-core / cross-cutting)
|
||||
- A shared "render-trusted-markdown" frontend helper (marked + DOMPurify
|
||||
in one place) so no future sink reintroduces F1. Not flotilla-core
|
||||
(it's rfc-app frontend), but a cross-cutting rfc-app refactor worth a
|
||||
roadmap item.
|
||||
- A security-headers block as a reusable nginx snippet shipped with the
|
||||
deploy/ templates — could become a flotilla overlay default.
|
||||
@@ -0,0 +1,112 @@
|
||||
# SESSION-0026.5 — flotilla operator-CLI security audit (READ-ONLY subsession)
|
||||
|
||||
- **Parent:** SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
- **Role:** subsession 0026.5 of OHM security-audit driver session 0026
|
||||
- **Surface:** `/Users/benstull/projects/wiggleverse/ohm-rfc-app-flotilla` (the flotilla operator CLI)
|
||||
- **Scope:** READ-ONLY. No edits, no deploys, no secret rotation, no git mutation.
|
||||
Only file written = this transcript.
|
||||
- **Focus:** §3 invariant 1 ("no secret bytes in the build pipeline / state / log /
|
||||
stdout"), overlay-vs-secret separation, SSH command safety, `secret set` stdin-only.
|
||||
|
||||
## Pre-state
|
||||
|
||||
Branch `main`, clean except untracked `.claude/`. Audited the canonical tree under
|
||||
`ohm_rfc_app_flotilla/` (NOT the `.claude/worktrees/` copies, which a naive `find`
|
||||
surfaced first — pivoted to the real package dir). Read SPEC.md §1–§13 + CLAUDE.md.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Read SPEC.md (§3 invariants, §6 overlay, §7 secrets, §8 deploy gesture, §10 log)
|
||||
and the binding CLAUDE.md "never ask for secret bytes" / two-layer rules.
|
||||
2. Listed canonical source; read secrets.py, ssh.py, deploy.py in one batch.
|
||||
3. Read deploy_log.py, db.py, registry.py, plan.py, pin.py.
|
||||
4. Grepped cli.py for stdin/tty/secret/overlay output paths; read the overlay +
|
||||
secret CLI verbs in full; read `secret set` (TTY guard + stdin).
|
||||
5. Checked migrations for any `secret_value` column (none — confirmed) and skimmed
|
||||
test_secrets.py + test_deploy.py for hardcoded/echoed real secrets (none; all
|
||||
stub bytes like `b"NEVER-LEAK-..."`).
|
||||
6. Adversarial pass on candidate findings:
|
||||
- KILLED "snapshot hash leaks overlay values" — hash is sha256, one-way; values
|
||||
not recoverable. Not a leak.
|
||||
- KILLED "SSH command injection via target_version" — pin.py validates strict
|
||||
SemVer regex `^\d+\.\d+\.\d+$` before it becomes `v<ver>`; all VM coords and
|
||||
vite values go through `shlex.quote`. No injection.
|
||||
- KILLED "secret could be set as overlay" as a *code bug* — downgraded to
|
||||
informational: overlay/secret are distinct tables + distinct CLI paths; the
|
||||
non-secret-overlay boundary is operator discipline per CLAUDE.md two-layer
|
||||
rule, and there's no automated key-name guard, but that's a documented design
|
||||
boundary, not a defect. Noted F3.
|
||||
- CONFIRMED the phase-detail redaction gap (F1) by tracing append_phase →
|
||||
persisted `phases[].detail` vs. the later error_summary-only redaction.
|
||||
- Held F2 (gcloud `--quiet` host-key TOFU) as needs-verification.
|
||||
|
||||
## Cut state — findings
|
||||
|
||||
- **F1 (MEDIUM, high confidence):** Per-phase `detail` is persisted to the
|
||||
`deploys.phases` JSON column UNREDACTED. deploy.py `_PhaseRunner.run` writes
|
||||
`detail=e.detail` (e.g. phase-3 `f"git fetch/checkout failed: {r.stderr...[:240]}"`)
|
||||
via `deploy_log.append_phase` at the moment of failure. The `redact_live` scrub
|
||||
runs LATER and ONLY on the separate `error_summary` string (`_finalize_failure`).
|
||||
A secret that surfaces in a command's stderr lands unredacted in flotilla state.
|
||||
Violates §3 invariant 1 / §10.2. The existing test (test_deploy.py:248) checks
|
||||
only `outcome.error_summary`, never the persisted `phases[].detail` — so the gap
|
||||
is untested. Remediation: redact phase detail at `append_phase` time (pass the
|
||||
live `_ResolvedSecrets` into `_PhaseRunner`), or redact in `_phaseN` before
|
||||
raising `_PhaseFailure`.
|
||||
|
||||
- **F2 (LOW, needs-verification):** ssh.py invokes `gcloud compute ssh ... --quiet`.
|
||||
`--quiet` auto-accepts prompts including first-connect host-key confirmation
|
||||
(TOFU silently accepted). gcloud still manages `google_compute_known_hosts` and
|
||||
pulls host keys from instance metadata when present, so steady-state MITM exposure
|
||||
is limited; residual risk is first-connect. Verify exact gcloud host-key behavior
|
||||
before treating as actionable. Remediation if confirmed: rely on GCP guest-attr
|
||||
host keys / IAP (the §19.2 "harden SSH via IAP" candidate already tracks this).
|
||||
|
||||
- **F3 (INFORMATIONAL):** No code guard prevents an operator from placing a
|
||||
sensitive value into the non-secret overlay (e.g. as a `VITE_*` key, which
|
||||
deploy.py phase-5 interpolates into the build command AND bundle-embeds). This is
|
||||
the documented two-layer boundary (operator discipline), not a defect — flagged
|
||||
so the driver knows the enforcement is human, not mechanical.
|
||||
|
||||
- **F4 (LOW, edge):** `redact()` / `redact_live()` match needles via
|
||||
`bytes.decode("utf-8", errors="replace")`. A secret containing non-UTF-8 bytes
|
||||
would decode to a different string than what landed in (replace-decoded) stderr,
|
||||
so redaction could miss it. OHM secrets are random ASCII/UTF-8 tokens, so low
|
||||
real risk; noted for completeness.
|
||||
|
||||
## What's solid (controls correctly enforcing §3 invariant 1)
|
||||
|
||||
- No `secret_value` column anywhere; migration 002 `secret_refs` holds only
|
||||
project/id/version refs. DB stores references, not bytes.
|
||||
- `secret set` is stdin-only with an explicit `sys.stdin.isatty()` TTY refusal
|
||||
(cli.py) — matches CLAUDE.md exactly; bytes never enter shell history/args.
|
||||
- `read_secret` returns bytes that the deploy/plan paths use transiently;
|
||||
`_ResolvedSecrets` holds them in bytearrays scrubbed-to-zero on scope exit
|
||||
(incl. exception unwind), env_body bytearray zeroed in a `finally`.
|
||||
- `.env` written atomically over SSH stdin (never local disk, never a command arg):
|
||||
`umask 077` + `cat > tmp` + `mv` as `sudo -u <user>`, mode 0600, owner = service
|
||||
user. Verified env body travels via `input=` (stdin), not the command string
|
||||
(test_deploy.py:166 asserts bytes are in stdin, not in any recorded command).
|
||||
- Snapshot hash and plan render use ONLY ref strings, never bytes.
|
||||
- error_summary redaction path IS applied (the F1 gap is the *phase detail*, a
|
||||
separate persisted field).
|
||||
- All SSH-interpolated values `shlex.quote`d; pin is strict-SemVer-validated.
|
||||
- `secret list` / `overlay show` render `[secret: <project>/<id>@<version>]`, never
|
||||
resolved bytes.
|
||||
- Tests use only stub bytes (`b"NEVER-LEAK-..."`, `b"abc123"`); no real secret
|
||||
hardcoded or echoed.
|
||||
|
||||
## Driver needs to know
|
||||
|
||||
- F1 is the one worth fixing: it's a real (if narrow — requires a secret to appear
|
||||
in remote stderr) §3-invariant-1 leak into flotilla state, and it's untested.
|
||||
- F2 ties into the already-tracked §19.2 "harden SSH via IAP" item.
|
||||
|
||||
## §19.2 candidates surfaced
|
||||
|
||||
- Mechanical overlay-key denylist / secret-key collision guard (relates F3) — make
|
||||
the two-layer rule enforced, not just disciplined.
|
||||
- Redaction hardening: redact at every state-write boundary (phase detail included),
|
||||
not only at finalize (relates F1, F4).
|
||||
|
||||
No secret bytes appear anywhere in this transcript.
|
||||
@@ -0,0 +1,72 @@
|
||||
# SESSION-0026.6 TRANSCRIPT — OHM security audit subsession (systemd / nginx / TLS / GCP IAM / firewall / secrets)
|
||||
|
||||
- **Subsession:** 0026.6 of OHM security-audit driver session 0026
|
||||
- **Parent transcript:** SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
- **Start:** 2026-05-28T13-46 (America/Los_Angeles) **End:** 2026-05-28T13-52
|
||||
- **Mode:** READ-ONLY audit of LIVE infrastructure (project `wiggleverse-ohm`, host ohm.wiggleverse.org)
|
||||
- **Mandate surface:** systemd unit hardening, service user, file perms, nginx/TLS/headers, GCP IAM + firewall + secrets + VM SA/scopes + static IP, session-history secret-byte spot-check.
|
||||
- **No secret bytes appear anywhere in this transcript** (§3 invariant 1 / conversation corollary). `.env` was read for variable NAMES + perms/owner only; values never echoed.
|
||||
|
||||
---
|
||||
|
||||
## Pre-state
|
||||
|
||||
- gcloud authed (active accounts incl. ben.stull@wiggleverse.org), project `wiggleverse-ohm`.
|
||||
- Coordination: sessions 0022.0 and 0026.0 marked ACTIVE/INPROGRESS in ohm-session-history. 0022.0 was mid VM-rename (ohm-app -> ohm-rfc-app) + deploy. I observed only; did not interfere.
|
||||
- VM rename observed COMPLETE by sample time: instance `ohm-rfc-app` (us-central1-a, RUNNING, NAT 136.116.40.66); systemd unit `ohm-rfc-app.service` active (restarted ~1m41s before sampling — i.e. 0022's deploy had just landed); legacy `ohm-app.service` no longer present.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Claimed a session ID via `claim-session-id.sh --start 2026-05-28T13-46`. **NOTE/ARTIFACT:** as a *subsession* (0026.6) I should not have run the top-level claim — it computed + PUSHED a top-level `0027` placeholder (`SESSION-0027.0-...--INPROGRESS.md`) to the shared repo. This is a stray placeholder that needs retraction/finalization by the driver. (Read-only-infra mandate honored; this is the session-history repo, not infra. I did not push further changes to retract it to avoid racing 0022.0/0026.0.)
|
||||
2. Read `ssh.py` (gcloud compute ssh wrapper, no keys held, OS Login) + `registry.py` (deployment schema). `flotilla deployment show ohm-rfc-app` gave VM name/zone/user/install dir/unit.
|
||||
3. `gcloud compute instances list` + `describe`: default compute SA, default GCE scope set (NO cloud-platform), `enable-oslogin` instance metadata set, Shielded (vTPM + integrity ON, **Secure Boot OFF**), tags http-server/https-server.
|
||||
4. `gcloud projects get-iam-policy`: only ben.stull = owner + secretmanager.admin + secretAccessor; service agents only otherwise. **No Editor sprawl, no default-SA project role, no SA user-managed keys, no extra humans.** (Refuted the standard "default SA has Editor" finding — it has no project role here.)
|
||||
5. `gcloud compute firewall-rules list`: default-allow-ssh (22) and default-allow-rdp (3389) open to 0.0.0.0/0; http/https to world (expected); internal 10.128.0.0/9.
|
||||
6. `gcloud secrets list`: 7 `ohm-rfc-app-*` named secrets + 4 hash-named (likely Certbot/other). Per-secret `get-iam-policy` for several = EMPTY (no per-secret bindings) -> secret access is project-IAM-only (ben.stull). VM default SA has NO secretAccessor (consistent with flotilla deploy-time resolution via operator ADC).
|
||||
7. `gcloud compute addresses list`: static IP `ohm-app-ip` 136.116.40.66 IN_USE; USERS self-link still references `.../instances/ohm-app` (stale label after rename; IP correctly attached). Cosmetic.
|
||||
8. SSH read-only batch #1 (systemctl cat/status, unit file perms): unit hardening baseline present; backend `uvicorn --host 127.0.0.1 --port 8000 --proxy-headers --forwarded-allow-ips 127.0.0.1`.
|
||||
9. SSH batch #2 (`systemctl show` effective props, service user, app/.env/DB perms, `ss -tlnp`): see findings. `.env` 600 owner-only (solid); SQLite DB **644 world-readable**; CapabilityBoundingSet = full default set; several Protect*/Restrict*/SystemCallFilter unset.
|
||||
10. SSH batch #3 (`nginx -T` greps, site files): proxy_pass localhost, client_max_body_size 4M, certbot TLS1.2/1.3 + modern ciphers in server block; nginx.conf http-block line 34 lists TLSv1/1.1 (latent). No security `add_header` except Cache-Control. server_tokens commented out.
|
||||
11. Local TLS/header checks (openssl s_client, curl -sI): LE E7 ECDSA cert valid 2026-05-26..08-24, HTTP->HTTPS 301. **Adversarially verified TLS1.0 and TLS1.1 are REJECTED (alert 70), TLS1.2 OK** -> KILLED the "TLS1.0/1.1 enabled" finding as a live exposure (server-block override wins over http-block line 34; line 34 downgraded to latent/informational). Confirmed NO security headers (no HSTS/CSP/X-Frame-Options/nosniff/Referrer-Policy) on / and /api/health. Server version banner leaks `nginx/1.22.1`.
|
||||
12. SSH batch #4 (sshd -T, nginx 443 default, unattended-upgrades, fail2ban): sshd permitrootlogin no / passwordauth no / pubkey-only -> heavily mitigates world-open :22 (DOWNGRADED that finding). unattended-upgrades enabled. fail2ban inactive (low value given no password auth).
|
||||
13. Cloned ohm-session-history (30 transcripts), grepped for sk-ant/AKIA/private-key/ghp_/xox + secret-name=longvalue assignments: only `FILL_FROM_*` placeholders matched -> session-history secret-byte spot-check CLEAN.
|
||||
|
||||
## Cut state — findings
|
||||
|
||||
(severity / confidence after self-refutation)
|
||||
|
||||
- **F1 — SQLite DB world-readable.** `/opt/ohm-rfc-app/backend/data/ohm-app.db` mode `644` owner `ohm-rfc-app`. Any local user/process can read all app data (users, OAuth identities, sessions, content). Remediation: `chmod 640` (or 600) the DB + ensure umask in deploy. **MEDIUM, HIGH conf.** (mitigated: single-tenant host, few local users.) Filename still pre-rename `ohm-app.db` — cosmetic.
|
||||
- **F2 — No HTTP security headers.** No HSTS, CSP, X-Frame-Options, X-Content-Type-Options, Referrer-Policy on ohm.wiggleverse.org (verified via curl on / and /api/health; nginx has only `add_header Cache-Control` on static). Clickjacking + SSL-strip-on-first-visit + MIME-sniff exposure for an app doing OAuth/auth. Remediation: add HSTS (with preload once confident), X-Frame-Options DENY / CSP frame-ancestors, nosniff, Referrer-Policy in the ohm server block. **MEDIUM(-HIGH), HIGH conf.**
|
||||
- **F3 — systemd CapabilityBoundingSet not restricted.** Effective bounding set = full default (cap_sys_admin, cap_sys_module, cap_bpf, cap_sys_ptrace, etc.). Unit sets no `CapabilityBoundingSet=`. Real impact LOW (process runs non-root uid 999 + NoNewPrivileges=true, so no effective caps), but violates least-privilege. Remediation: `CapabilityBoundingSet=` (empty) — service binds 8000 so needs none. **LOW-MEDIUM, HIGH conf.**
|
||||
- **F4 — systemd deeper sandbox knobs unset.** PrivateDevices=no, ProtectKernelTunables=no, ProtectKernelModules=no, ProtectControlGroups=no, RestrictAddressFamilies unset, SystemCallFilter unset, LockPersonality=no, MemoryDenyWriteExecute=no, RestrictNamespaces=no. (Unit comment self-describes as "modest defaults.") Defense-in-depth gaps. Remediation: add the above (RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX; SystemCallFilter=@system-service; etc.). **LOW, HIGH conf.**
|
||||
- **F5 — Firewall: SSH(22) + RDP(3389) open to 0.0.0.0/0.** DOWNGRADED: SSH is well-mitigated (OS Login enabled instance-level + sshd passwordauth=no, permitrootlogin=no, pubkey-only) so it requires a valid GCP IAM identity; remaining risk is brute-force noise / SSH 0-day surface. RDP rule is pure dead surface on a Linux host (nothing listens on 3389). Remediation: delete default-allow-rdp; restrict :22 to IAP range (35.235.240.0/20) or known admin IPs, prefer --tunnel-through-iap. **LOW (SSH) / LOW (RDP), HIGH conf.**
|
||||
- **F6 — VM uses default compute service account.** `727327350842-compute@developer.gserviceaccount.com`. Mitigated: it holds NO project IAM role and NO secretAccessor, and scopes are the default set (no cloud-platform). Best practice is a dedicated minimal SA, but blast radius today is small. **LOW, HIGH conf.**
|
||||
- **F7 — Shielded VM Secure Boot OFF.** vTPM + integrity monitoring ON, Secure Boot OFF. Remediation: enable Secure Boot (requires stop/start; coordinate). **LOW, MED conf.**
|
||||
- **F8 — nginx version banner disclosure.** `Server: nginx/1.22.1` leaked; `server_tokens off;` is commented out. Remediation: enable `server_tokens off`. **LOW, HIGH conf.**
|
||||
- **F9 — nginx.conf http-block lists TLSv1/TLSv1.1 (line 34).** LATENT only — overridden by the certbot server-block (TLS1.0/1.1 verified rejected live). Risk: a future vhost without the certbot include would inherit weak protocols. Remediation: fix the http-block default to TLSv1.2 TLSv1.3. **INFORMATIONAL/LOW, HIGH conf.**
|
||||
- **F10 — Static IP / DB stale naming.** `ohm-app-ip` address USERS self-link references old `ohm-app` instance; DB file `ohm-app.db`. Cosmetic post-rename drift, no security impact. **INFO.**
|
||||
|
||||
### Killed / refuted
|
||||
- "TLS 1.0/1.1 enabled" — refuted: live handshake rejects both (alert 70).
|
||||
- "Default SA has project Editor" — refuted: default SA has no project-level role at all.
|
||||
- "Backend :8000 exposed" — refuted: binds 127.0.0.1 only; firewall has no :8000 rule.
|
||||
- "World-open SSH is high" — downgraded: OS Login + pubkey-only + no root login.
|
||||
- "Secrets readable by VM/others" — refuted: per-secret IAM empty, project-IAM grants accessor only to ben.stull.
|
||||
|
||||
### What's solid
|
||||
- `.env` 600 owner-only; service runs as dedicated nologin uid 999 with no extra groups, owns only its app dir.
|
||||
- Backend bound to 127.0.0.1:8000, nginx proxy_pass localhost, --forwarded-allow-ips 127.0.0.1, client_max_body_size 4M.
|
||||
- TLS: LE ECDSA cert valid, TLS1.2/1.3 only (verified), modern ciphers, session tickets off, HTTP->HTTPS 301.
|
||||
- sshd: pubkey-only, no root login, no password/empty-password auth; OS Login on.
|
||||
- IAM minimal (single owner, no SA keys, no editor sprawl); VM SA has no project role + no secretAccessor; default (not cloud-platform) scopes.
|
||||
- unattended-upgrades enabled.
|
||||
- ohm-session-history spot-check: no secret bytes across 30 transcripts (only FILL_FROM_* placeholders).
|
||||
- systemd baseline hardening present (NoNewPrivileges, ProtectSystem=strict, ProtectHome, PrivateTmp, ReadWritePaths scoped to data dir).
|
||||
|
||||
## Driver-needs-to-know
|
||||
- **Stray `0027` INPROGRESS placeholder** was pushed to ohm-session-history by my claim-script call (I should not have claimed as a subsession). Driver should retract/finalize it.
|
||||
- Top remediations by value: F1 (chmod DB 640) and F2 (security headers) first; both low-effort, real defense-in-depth wins. Then F3/F4 (systemd caps + sandbox) as a single unit edit, F5 (drop RDP rule, restrict/IAP SSH).
|
||||
- 0022's deploy had just landed at sample time; everything observed is post-rename steady state. No deploy was in progress during my SSH samples; no locks touched.
|
||||
|
||||
## §19.2 candidates (flotilla-core extraction)
|
||||
- A `flotilla harden check` verb could codify the systemd-unit + nginx-header + DB-perms assertions as a post-deploy audit (generic across deployments). The hardening expectations (F1–F4, F8) are deployment-shape-agnostic and would live in flotilla-core, with per-deployment exceptions in §13.
|
||||
@@ -0,0 +1,110 @@
|
||||
# SESSION-0026.7 — Secret-byte sweep of the OHM session-history corpus
|
||||
|
||||
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
Subsession of OHM security-audit driver session 0026.
|
||||
Role: READ-ONLY assurance pass. Mission: sweep every published session
|
||||
transcript (plus the local working transcripts that get published) for
|
||||
any leaked SECRET BYTE — the §3-invariant-1 conversation-layer corollary.
|
||||
|
||||
## Pre-state
|
||||
|
||||
Two scan surfaces:
|
||||
1. PUBLISHED corpus — cloned read-only to `/tmp/ohm-hist-scan-0026`
|
||||
from `ssh://git@git.wiggleverse.org:2222/wiggleverse/ohm-session-history.git`
|
||||
(clone exit 0; reachable).
|
||||
2. LOCAL working transcripts at `/Users/benstull/git/ohm-infra/SESSION-*.md`.
|
||||
|
||||
File counts:
|
||||
- Published: 29 transcript `.md` files (sessions 0001–0027, including
|
||||
the `.1/.2/.3` subsession files), plus `README.md`. = 30 `.md` total.
|
||||
- Local: 40 `SESSION-*.md` (includes INPROGRESS placeholders,
|
||||
SESSION-PROTOCOL.md, SESSION-L). Superset of published.
|
||||
|
||||
ABSOLUTE RULE held throughout: never reproduce a suspected secret value;
|
||||
report only pattern-name + location + masked form. No secret bytes were
|
||||
written here or in the report.
|
||||
|
||||
## Turn-by-turn arc
|
||||
|
||||
1. Cloned the published repo (exit 0) and listed both surfaces. Counted
|
||||
files.
|
||||
|
||||
2. **High-confidence shape sweep** (`grep -rniE`, both surfaces,
|
||||
`.git` excluded):
|
||||
- Provider prefixes: `sk-ant-`, `sk-…` (OpenAI), `AIza…` (Google),
|
||||
`ghp_/gho_/ghs_`, `glpat-` → **0 hits**.
|
||||
- PEM `BEGIN … PRIVATE KEY` → **0 hits**.
|
||||
- GCP SA JSON (`"private_key"`, `"private_key_id"`,
|
||||
`"type":"service_account"`) → **0 hits**.
|
||||
- JWT `eyJ….eyJ…` → **0 hits**.
|
||||
|
||||
3. **Named-secret-var = value sweep** (SECRET_KEY, GITEA_BOT_TOKEN,
|
||||
OAUTH_CLIENT_SECRET, GITEA_WEBHOOK_SECRET, etc. followed by `=`/`:`
|
||||
and a 6+ char value), plus a generic `*_TOKEN/_SECRET/_PASSWORD/_KEY
|
||||
= value` sweep. Hits were all adjudicated:
|
||||
- SESSION-0002 lines 950/961/967/971/974/1111 — placeholders:
|
||||
`FILL_FROM_PASSWORD_MANAGER`, `FILL_FROM_OPENSSL_OUTPUT_ABOVE`, and
|
||||
a literal `openssl rand -hex 32` gesture, and a doc example
|
||||
`GITEA_BOT_TOKEN=abc123...`. Not real values. CLEAN.
|
||||
- SESSION-0008 line 316 — a shell `grep '^OAUTH_CLIENT_SECRET='`
|
||||
command line, no value. CLEAN.
|
||||
- SESSION-0008 lines 684/685 — the CRLF-bug writeup showing
|
||||
`OAUTH_CLIENT_SECRET=" gto_oge…\r"` and `SECRET_KEY="2e05383a849…\r"`.
|
||||
Opened lines 665–709. Both values are **deliberately truncated with
|
||||
an ellipsis** (`…`): only a ~3-char prefix of the gitea token and
|
||||
~11 hex chars of the 64-char SECRET_KEY are shown, as illustration
|
||||
of the trailing-`\r` taint. The remaining bytes are NOT present.
|
||||
Verified `gto_oge` appears at lines 70/291/684/689 and is the same
|
||||
truncated 3-char suffix every time — never full length. CLEAN
|
||||
(truncated, sub-threshold).
|
||||
- SESSION-0001 line 984 — `GITEA_SIGNING_KEY="7C9E…1592E2"` (40-hex).
|
||||
Opened lines 972–993: it's inside `/tmp/gitea-install.sh`, used with
|
||||
`gnupg`/`dirmngr` to verify the downloaded Gitea release signature.
|
||||
This is the **public GPG key fingerprint** Gitea publishes. Public
|
||||
by definition. CLEAN.
|
||||
|
||||
4. **High-entropy string sweep** (standalone 48+ char base64/hex blobs in
|
||||
published corpus). Two non-trivial strings, both adjudicated:
|
||||
- 64-hex `15783e…574d` (SESSION-0006 line 503) — explicitly labeled
|
||||
`overlay snapshot sha256:…`. A content hash of the (non-secret)
|
||||
overlay config. CLEAN.
|
||||
- `AAAAC3NzaC1lZDI1NTE5…` (SESSION-0001 line 683) — the standard
|
||||
`ssh-ed25519` **public-key** wire-format prefix. Public key, not a
|
||||
private key. CLEAN. (Consistent with the PEM sweep finding zero
|
||||
`BEGIN OPENSSH PRIVATE KEY` blocks.)
|
||||
|
||||
5. **Final confirmation sweeps:**
|
||||
- bcrypt `$2[aby]$NN$` → **0 hits**.
|
||||
- `password/passwd/pass/pwd = value` (excluding prose, placeholders,
|
||||
`--password` flags, "password manager") → **0 hits**.
|
||||
- README.md → only prose about the no-secret-bytes policy. CLEAN.
|
||||
- `PRIVATE KEY / BEGIN OPENSSH / BEGIN RSA / BEGIN EC` → all matches
|
||||
are **prose discussing** private keys (e.g. "client unlocking that
|
||||
private key", "No private key material … nothing sensitive", and a
|
||||
grep-pattern example inside a transcript). No actual key blocks.
|
||||
CLEAN.
|
||||
|
||||
## Cut state — VERDICT: CLEAN
|
||||
|
||||
Zero secret bytes leaked across the entire corpus.
|
||||
- 29 published transcript `.md` files scanned (+README).
|
||||
- 40 local `SESSION-*.md` files scanned (published superset).
|
||||
- The only "value-shaped" hits were all expected/safe: placeholders,
|
||||
a public GPG fingerprint, a public ssh-ed25519 key, a sha256 overlay
|
||||
snapshot hash, doc examples, and two deliberately-ellipsis-truncated
|
||||
illustration values in the CRLF-bug writeup (sub-threshold, remainder
|
||||
never present). No secret REFERENCES were mistaken for bytes; no
|
||||
references were flagged as leaks.
|
||||
|
||||
## Driver needs to know
|
||||
|
||||
- Corpus is CLEAN. No remediation needed. The §3-invariant-1
|
||||
conversation-layer corollary is holding in practice across all
|
||||
published sessions to date.
|
||||
- One thing worth a glance (NOT a leak): SESSION-0008's CRLF-bug
|
||||
narrative quotes truncated `…`-elided forms of OAUTH_CLIENT_SECRET and
|
||||
SECRET_KEY. They are safely below any useful threshold, but if the
|
||||
team wants zero value-fragments at all, those two could be masked to
|
||||
pattern-name only. Optional, low priority.
|
||||
- Published repo was reachable and freshly cloned; no "could not reach"
|
||||
caveat applies.
|
||||
@@ -0,0 +1,138 @@
|
||||
# SESSION-0026.8 — v0.25.0 security hardening config edits
|
||||
|
||||
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
|
||||
Subsession 0026.8 of OHM session 0026 (REMEDIATION mode). Scope was
|
||||
narrow and bounded: make exactly two in-repo config edits in the
|
||||
`rfc-app` repo (branch `feature/v0.25.0-security-hardening`) to close
|
||||
audit findings M2, L8 (nginx) and L4 (systemd). No git commit, no
|
||||
push, no VM access, no other files touched. The driver commits and
|
||||
applies to the VM later.
|
||||
|
||||
## What I changed and why
|
||||
|
||||
### TASK 1 — nginx security headers (M2 + L8)
|
||||
File: `deploy/nginx/ohm.wiggleverse.org.conf`
|
||||
|
||||
Added to the server block (which is the certbot-promoted HTTPS block on
|
||||
the VM — see below): `server_tokens off;` (L8) plus five `add_header
|
||||
... always;` response headers (HSTS, X-Frame-Options DENY,
|
||||
X-Content-Type-Options nosniff, Referrer-Policy
|
||||
strict-origin-when-cross-origin, and a Content-Security-Policy) (M2).
|
||||
All carry `always` so they apply to nginx error responses too.
|
||||
|
||||
Important structural note I verified before editing: the in-repo
|
||||
`.conf` is the **pre-certbot template** and contains only the port-80
|
||||
`server` block. Per the file's own install comment, RUNBOOK.md, and
|
||||
DEPLOY-NEW-SESSION-PROMPT.md, `certbot --nginx` rewrites this file on
|
||||
the VM, promoting this single server block to `listen 443 ssl` and
|
||||
adding a separate port-80 → 443 redirect block. So headers placed in
|
||||
this block ride into the HTTPS server block on the VM exactly as the
|
||||
task intended. I documented that relationship in an inline comment so a
|
||||
future reader isn't confused about why "the HTTPS headers" live in what
|
||||
looks like the port-80 block in-repo.
|
||||
|
||||
Did NOT touch the existing TLS/proxy/`Cache-Control`/`client_max_body_size`
|
||||
directives.
|
||||
|
||||
### TASK 2 — systemd hardening (L4)
|
||||
File: `deploy/systemd/rfc-app.service`
|
||||
|
||||
Kept the existing NoNewPrivileges / ProtectSystem=strict / ProtectHome
|
||||
/ PrivateTmp / scoped ReadWritePaths. Added the full defense-in-depth
|
||||
block under a `# v0.25.0 security hardening (audit 0026 L4)` comment:
|
||||
empty `CapabilityBoundingSet=` + `AmbientCapabilities=` (service binds
|
||||
127.0.0.1:8000, needs no caps), PrivateDevices, ProtectKernel{Tunables,
|
||||
Modules,Logs}, ProtectControlGroups, RestrictAddressFamilies (AF_INET
|
||||
AF_INET6 AF_UNIX), RestrictNamespaces, LockPersonality,
|
||||
MemoryDenyWriteExecute, RestrictRealtime, RestrictSUIDSGID,
|
||||
SystemCallFilter=@system-service, SystemCallErrorNumber=EPERM,
|
||||
SystemCallArchitectures=native, UMask=0077.
|
||||
|
||||
## The CSP I settled on + reasoning
|
||||
|
||||
```
|
||||
default-src 'self';
|
||||
script-src 'self' https://challenges.cloudflare.com https://*.amplitude.com;
|
||||
style-src 'self' 'unsafe-inline';
|
||||
img-src 'self' data: https:;
|
||||
font-src 'self' data:;
|
||||
connect-src 'self' https://*.amplitude.com https://challenges.cloudflare.com;
|
||||
worker-src 'self' blob:;
|
||||
frame-src https://challenges.cloudflare.com;
|
||||
frame-ancestors 'none';
|
||||
base-uri 'self';
|
||||
object-src 'none'
|
||||
```
|
||||
|
||||
Diffs from the suggested starting policy, all evidence-driven from
|
||||
reading the frontend:
|
||||
|
||||
- **Added `connect-src https://challenges.cloudflare.com`.** Turnstile
|
||||
(TurnstileWidget.jsx loads `https://challenges.cloudflare.com/turnstile/v0/api.js`)
|
||||
makes XHR/fetch calls back to that origin during the challenge, not
|
||||
just an iframe. Without it the challenge can fail.
|
||||
|
||||
- **Added `worker-src 'self' blob:`.** This is the load-bearing change.
|
||||
Amplitude analytics is `@amplitude/unified`, which ships **Session
|
||||
Replay**, and analytics.js initializes it at **`sampleRate: 1`** (100%
|
||||
of consenting sessions recorded — see lib/analytics.js lines 219–222).
|
||||
Amplitude Session Replay spins up a Web Worker from a `blob:` URL for
|
||||
DOM capture/compression. With a strict `default-src 'self'` and no
|
||||
`worker-src`, `blob:` workers are blocked and session replay breaks
|
||||
for every consenting user. I could not introspect node_modules to
|
||||
prove the exact blob usage (deps not installed in this checkout), so
|
||||
I relied on documented Amplitude Session Replay behavior and added
|
||||
`blob:` defensively — it's low-risk and prevents a real regression.
|
||||
|
||||
- **script-src / connect-src amplitude:** Amplitude AND mermaid are
|
||||
both **bundled** (`import('@amplitude/unified')`, `import('mermaid')`
|
||||
— dynamic imports from node_modules, served from `'self'`), so
|
||||
script-src for them isn't strictly required. I kept
|
||||
`script-src ... https://*.amplitude.com` defensively (harmless) but
|
||||
the truly necessary amplitude grant is `connect-src https://*.amplitude.com`
|
||||
(event ingestion + session replay upload, all under `*.amplitude.com`).
|
||||
mermaid needs no external origin at all — it renders inline SVG.
|
||||
|
||||
- **`style-src 'unsafe-inline'` retained and documented as required:**
|
||||
the JSX uses inline `style={...}` attributes throughout and mermaid
|
||||
injects `<style>` at render time. Tightening to nonces/hashes is a
|
||||
build-side follow-up. **`script-src` deliberately omits
|
||||
`'unsafe-inline'`** — no inline `<script>` in the app (index.html has
|
||||
only `<script type="module" src="/src/main.jsx">`), so inline-script
|
||||
XSS stays blocked.
|
||||
|
||||
- `img-src 'self' data: https:` and `font-src 'self' data:` keep
|
||||
markdown/RFC remote images and data: URIs (and svg/mermaid data:
|
||||
output) working.
|
||||
|
||||
## MemoryDenyWriteExecute judgment call
|
||||
|
||||
I included `MemoryDenyWriteExecute=true` **active, not commented out**.
|
||||
The stack is stock CPython + FastAPI/uvicorn + sqlite3 + bcrypt + httpx
|
||||
— no JIT, no PyPy, nothing that mmaps W+X pages. MDWX is safe for that
|
||||
profile. The risk is theoretical (a C-extension doing W+X mmap), so I
|
||||
judged the security win worth keeping it on. I added an inline comment
|
||||
flagging it as the single line to comment out if uvicorn fails to come
|
||||
up on restart. Driver action: **watch the first `systemctl restart
|
||||
rfc-app` journal** — if uvicorn crashes on boot, comment out the one
|
||||
`MemoryDenyWriteExecute=true` line and `daemon-reload` + restart;
|
||||
everything else in the block is restart-safe for this stack.
|
||||
|
||||
## Verify
|
||||
|
||||
Re-read both files after editing.
|
||||
- nginx: brace-balanced (1 `server {` + 4 `location {` = 5 opens, 5
|
||||
closes). Could not run `nginx -t` (no live nginx) — visual sanity
|
||||
check only; CSP is a single quoted `add_header` value, well-formed.
|
||||
- systemd: valid `key=value` ini throughout; the two empty-value
|
||||
capability clears (`CapabilityBoundingSet=`, `AmbientCapabilities=`)
|
||||
are intentional and valid systemd syntax.
|
||||
|
||||
No secret bytes touched or printed. No commit/push/VM action taken.
|
||||
|
||||
## Cut state
|
||||
|
||||
Both edits complete and verified in-repo on
|
||||
`feature/v0.25.0-security-hardening`. Handed back to driver for commit
|
||||
+ VM apply. Open driver watch-item: MDWX on first restart (above).
|
||||
@@ -0,0 +1,76 @@
|
||||
# SESSION-0026.9 — flotilla audit finding L1 remediation
|
||||
|
||||
Parent: SESSION-0026.0-TRANSCRIPT-2026-05-28T13-46--INPROGRESS.md
|
||||
|
||||
Mode: REMEDIATION (operator authorized). Subsession of OHM session 0026.
|
||||
Scope: fix flotilla audit finding L1 in
|
||||
`ohm-rfc-app-flotilla/ohm_rfc_app_flotilla/deploy.py` + a test.
|
||||
No commit, no push, no VERSION bump, no VM/deploy actions.
|
||||
|
||||
## Finding L1 (recap)
|
||||
|
||||
`_PhaseRunner.run` persisted each phase's `detail` (which can embed a
|
||||
failed command's `stderr[:240]`) into the `deploys.phases` JSON column
|
||||
via `deploy_log.append_phase` WITHOUT redaction. Secret redaction
|
||||
(`_ResolvedSecrets.redact_live`) was applied only to the separate
|
||||
`error_summary` field in `_finalize_failure`. A secret surfacing in a
|
||||
command's stderr would land unredacted in flotilla state — a §3
|
||||
invariant 1 / §10.2 leak.
|
||||
|
||||
## Design choice — how the redactor was threaded
|
||||
|
||||
- Added an OPTIONAL field `redactor: _ResolvedSecrets | None = None` to
|
||||
the `_PhaseRunner` dataclass, plus a small `_redact(text)` helper that
|
||||
calls `redactor.redact_live(text)` when the redactor is present and
|
||||
falls back to the raw text when it's None.
|
||||
- Chose OPTIONAL over REQUIRED because it's the lower-churn option:
|
||||
it preserves the ability to construct a `_PhaseRunner` without a
|
||||
resolved-secrets context (none of the current tests do, but the
|
||||
nullable style matches the rest of the module's injected-dependency
|
||||
shape), and required no signature changes at any other call site.
|
||||
- `run_deploy` constructs the only production runner and now passes
|
||||
`redactor=resolved`. Phases run inside the `_ResolvedSecrets`
|
||||
`with`/try scope, so the bytearrays are still live when `_redact`
|
||||
runs during the phase — redaction happens before `scrub()` zeroes
|
||||
them at scope exit, exactly as the finding requires.
|
||||
- Both the success-path detail and the failure-path detail are now
|
||||
redacted before they reach the `PhaseRecord` (persisted JSON) AND
|
||||
before the `on_progress(...)` strings that interpolate the detail
|
||||
(those can reach operator terminals/logs).
|
||||
- `_finalize_failure` still redacts the summary it builds from the
|
||||
last phase's detail; since that detail is now already redacted, the
|
||||
second pass is idempotent (no behavior change).
|
||||
|
||||
## Diff summary
|
||||
|
||||
deploy.py:
|
||||
- `_PhaseRunner`: new `redactor` field + `_redact` helper; `run`
|
||||
computes `safe_detail = self._redact(...)` on both paths and uses it
|
||||
for the `PhaseRecord.detail`, the `on_progress` FAILED message, and
|
||||
the returned detail.
|
||||
- `run_deploy`: `_PhaseRunner(...)` construction now passes
|
||||
`redactor=resolved`.
|
||||
|
||||
tests/test_deploy.py:
|
||||
- Extended `test_run_deploy_redacts_secret_bytes_from_error_summary`
|
||||
to ALSO load the persisted row and assert the failing phase's
|
||||
`detail` contains `[REDACTED]` and not the stub secret, and that no
|
||||
phase detail anywhere carries the raw stub bytes.
|
||||
|
||||
## Test added / extended
|
||||
|
||||
Reused the existing redaction test (which drives a real `run_deploy`
|
||||
with a `LeakyFail` ssh stub that vomits a fake secret into stderr) and
|
||||
added the `deploys.phases` assertions. Stub value is the module's
|
||||
`STUB_SECRET_BYTES = b"NEVER-LEAK-stub-secret-payload"` — a fake, never
|
||||
a real secret.
|
||||
|
||||
## Test-run result
|
||||
|
||||
`.venv/bin/pytest tests/ -q` → 157 passed in 0.28s. Suite was green
|
||||
before the change (full-suite run, not a known-broken baseline).
|
||||
|
||||
## Cut state
|
||||
|
||||
Code change + test in place. No commit, no push, no VERSION bump, no
|
||||
VM/deploy action. Driver handles release.
|
||||
Reference in New Issue
Block a user