fix(security): token CSPRNG + proxy-trust + og:image hardening (LC-155) #191

Merged

nrupard merged 2 commits from feat/lc-155-token-csprng-hardening into main 2026-05-22 21:29:30 +02:00

Conversation 0 Commits 2 Files changed 6 +118 -23

nrupard commented 2026-05-22 21:21:01 +02:00

Owner

Copy link

What

Defense-in-depth hardening bundle from the LC-148 audit (S10-S12).

• S10 - explicit CSPRNG. Session-token and API-token generation (the latter also backing the incoming-webhook secret, which reuses generate_api_token) now draw from rand::rngs::OsRng explicitly. thread_rng is already CSPRNG-backed, so this is not a live vuln; the point is to put the guarantee in the source rather than have it be incidental to the default RNG. Scoped to the security tokens the audit named (not the wider set of thread_rng callers, which are equally CSPRNG-backed today).
• S11 - proxy-header trust. extract_session_origin only trusts the X-Forwarded-For / X-Real-IP client IP when the deployment opts into proxy headers; otherwise the stored session / login-alert IP is None rather than a forgeable value. New shared auth::proxy_headers_trusted helper (reads trust_proxy_headers) used by both the session-origin extractor and the rate limiter, so both honor one switch. User-Agent is still always captured (display-only).
• S12 - og:image scheme. The remote og:image is resolved against the final page URL and kept only when it is an absolute http/https URL, so a page can't put a javascript: / data: / other-scheme value into an <img src>. Dropped (not neutralized) when disallowed.

Test

sanitize_image_url unit tests (absolute + relative http(s) accepted; javascript:/data:/vbscript:/file: rejected). just check / just test / just test-saas green (the routes_uploads flake under concurrent-binary load is pre-existing and passes in isolation; no upload code touched).

Completes the security findings in the LC-159 post-audit story (LC-149 through LC-155).

🤖 Generated with Claude Code

## What Defense-in-depth hardening bundle from the LC-148 audit (S10-S12). - **S10 - explicit CSPRNG.** Session-token and API-token generation (the latter also backing the incoming-webhook secret, which reuses `generate_api_token`) now draw from `rand::rngs::OsRng` explicitly. `thread_rng` is already CSPRNG-backed, so this is not a live vuln; the point is to put the guarantee in the source rather than have it be incidental to the default RNG. Scoped to the security tokens the audit named (not the wider set of `thread_rng` callers, which are equally CSPRNG-backed today). - **S11 - proxy-header trust.** `extract_session_origin` only trusts the `X-Forwarded-For` / `X-Real-IP` client IP when the deployment opts into proxy headers; otherwise the stored session / login-alert IP is `None` rather than a forgeable value. New shared `auth::proxy_headers_trusted` helper (reads `trust_proxy_headers`) used by both the session-origin extractor and the rate limiter, so both honor one switch. User-Agent is still always captured (display-only). - **S12 - og:image scheme.** The remote `og:image` is resolved against the final page URL and kept only when it is an absolute http/https URL, so a page can't put a `javascript:` / `data:` / other-scheme value into an `<img src>`. Dropped (not neutralized) when disallowed. ## Test `sanitize_image_url` unit tests (absolute + relative http(s) accepted; `javascript:`/`data:`/`vbscript:`/`file:` rejected). `just check` / `just test` / `just test-saas` green (the `routes_uploads` flake under concurrent-binary load is pre-existing and passes in isolation; no upload code touched). Completes the security findings in the LC-159 post-audit story (LC-149 through LC-155). 🤖 Generated with [Claude Code](https://claude.com/claude-code)

nrupard added 1 commit 2026-05-22 21:21:01 +02:00

fix(security): token CSPRNG + proxy-header trust + og:image scheme hardening (LC-155) ... b6735ebde5

Defense-in-depth bundle from the audit (S10-S12).

S10: session-token and API-token (also the incoming-webhook secret, which reuses generate_api_token) generation now draw from rand::rngs::OsRng explicitly. thread_rng is already CSPRNG-backed, but for a security token the guarantee should be in the source, not incidental to the default RNG.

S11: extract_session_origin only trusts the X-Forwarded-For / X-Real-IP client IP when the deployment opts into proxy headers; otherwise the stored session/login-alert IP is None rather than a forgeable value. Adds a shared auth::proxy_headers_trusted helper (reads the trust_proxy_headers setting) used by both the session-origin extractor and the rate limiter, so both honor one switch. The User-Agent is still always captured (display-only, not a security decision).

S12: the remote og:image is resolved against the final page URL and kept only when it is an absolute http/https URL, so a page cannot put a javascript:/data:/other-scheme value into an <img src>. Dropped (not neutralized) when disallowed; a preview without an image is fine.

Tests: sanitize_image_url accepts absolute + relative http(s), rejects javascript:/data:/vbscript:/file:.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard added 1 commit 2026-05-22 21:28:31 +02:00

fix(unfurl): re-sanitize cached og:image on the cache-hit path (LC-155) ... 9b35a25c0f

Review follow-up: the cache-hit early return rendered row.image_url straight from the DB, so a row written before the og:image scheme guard could still surface a non-http(s) image source. Re-run sanitize_image_url (resolved against the row's URL) on read so stale rows are cleaned too. Low risk (Askama escapes the attribute and an img src does not execute), but keeps the guard consistent across both render paths.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

nrupard merged commit b4ff9cc525 into main 2026-05-22 21:29:30 +02:00

nrupard referenced this pull request from a commit 2026-05-22 21:29:31 +02:00

Merge pull request 'fix(security): token CSPRNG + proxy-trust + og:image hardening (LC-155)' (#191) from feat/lc-155-token-csprng-hardening into main

nrupard deleted branch feat/lc-155-token-csprng-hardening 2026-05-22 21:29:31 +02:00

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels

No items

No labels

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

psa-systems/lets-chat!191

No description provided.