Skip to content

fix(security): close the five high-severity audit findings - #179

Merged
s3rot0n1n merged 3 commits into
mainfrom
fix/security-highs
Nov 3, 2026
Merged

s3rot0n1n merged 3 commits into
mainfrom
fix/security-highs

Conversation

@s3rot0n1n

Copy link
Copy Markdown
Contributor

Summary

Fixes the five high-severity findings from the second security-audit run against main (620f98f).

  1. CSRF on cookie-session writes: the REST API now requires the dashboard origin plus the double-submit `X-Sendlit-CSRF` token for session writes. The dashboard proxy checks same-origin and attaches the token itself, so dashboard clients are unchanged.
  2. OAuth audience confusion: tokens issued for the MCP resource no longer authenticate REST (`401 invalid_token`).
  3. MCP ESP / Postmaster / SNDS tools: now enforce the same team-admin rule as REST via a shared `team/authority.ts` (team keys allowed; ESP changes also need team ESPs enabled).
  4. Dashboard `/api/proxy`: every response gets `Content-Security-Policy: default-src 'none'; sandbox` and `nosniff`, so tenant HTML can't run on the dashboard origin.
  5. Org removal/demotion: removing a member, or demoting an admin to member, revokes their team memberships and the team API keys they created.

Docs (authentication, MCP) updated. Each fix has a regression test.

Behaviour to confirm

Demoting an org admin to member also removes access to teams they were explicitly added to.

Test plan

  • API: `bun test --parallel` 1029 pass / 0 fail
  • Web proxy tests pass
  • QA smoke test (dashboard writes with CSRF token, MCP-only token refused on REST, member forbidden on MCP ESP tools)

🤖 Generated with Claude Code

- CSRF: cookie-session writes on the REST API now require the dashboard
  origin and the double-submit CSRF token; the dashboard proxy checks the
  origin and attaches the token itself.
- OAuth tokens issued for the MCP resource no longer authenticate REST.
- MCP ESP and Postmaster/SNDS tools enforce the same team-admin rule as REST
  (shared in team/authority.ts).
- The dashboard /api/proxy sandboxes every response (CSP + nosniff) so tenant
  HTML cannot run on the dashboard origin.
- Removing or demoting an organization member revokes their team access and
  the team API keys they created.

Docs for authentication and MCP updated.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-03T08:16:58.505451Z de05bd9 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@s3rot0n1n

Copy link
Copy Markdown
Contributor Author

QA smoke test on qa-fix-security-highs-afa416f (arm64, deployed to sendlit-qa):

  • Session write with no/evil origin → 403 csrf_origin_invalid; good origin without token → 403 csrf_token_invalid; good origin + matching token → accepted.
  • Dashboard proxy end-to-end (same-origin POST /contacts) works; cross-origin and no-origin → 403; proxied responses carry CSP: default-src 'none'; sandbox + nosniff.
  • OAuth token issued for /mcp → REST 401 invalid_token; same token still initialises on /mcp.
  • Not exercised on QA (unit-tested only): member forbidden on MCP ESP/Postmaster tools, org removal/demotion revoking team access.

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@s3rot0n1n
s3rot0n1n merged commit 2b21858 into main Nov 3, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant