Skip to content

Fix five verified security findings from the audit - #24

Open
vijaythecoder wants to merge 8 commits into
mainfrom
claude/loving-benz-097c19
Open

vijaythecoder wants to merge 8 commits into
mainfrom
claude/loving-benz-097c19

Conversation

@vijaythecoder

Copy link
Copy Markdown
Collaborator

Summary

Implements fixes for the five findings verified in the recent security audit, one commit per finding, each covered by new Pest feature tests.

  1. HIGH — MIME header injection via CRLF in custom header names. SendEmailRequest now requires header names to match RFC 5322 ftext (/^[A-Za-z0-9-]+$/) in addition to the reserved-name blocklist, covering both the API and dashboard send paths. As defense in depth, MimeMessageBuilder::build() rejects any custom header whose name or value contains CR or LF.

  2. MEDIUM — SNS webhook accepted AWS-signed messages from any AWS account. New nullable sns_topic_arn column on sources; the TopicArn from the first confirmed SNS subscription is pinned (first confirmation wins) and any later envelope — confirmation or notification — signed for a different topic is rejected. Envelopes with a Timestamp older than 15 minutes, more than 5 minutes in the future, or unparseable are rejected as replays. All rejections are logged to WebhookLog with status rejected.

  3. MEDIUM — login throttle bypass via trustProxies(at: '*'). Trusted proxies are now configured via a new TRUSTED_PROXIES env variable (comma-separated IPs/CIDRs, default none, documented in .env.example), and the Fortify login limiter keys on the socket address (REMOTE_ADDR), matching the existing larasend-api-ip limiter, so rotating X-Forwarded-For values can no longer reset the throttle.

  4. MEDIUM — workspace-context hijack via consentless member attachment. The active workspace now resolves to a workspace the user owns before falling back to membership order (in both ProjectContext::workspaceFor() and the HandleInertiaRequests settings navigation), and users can remove themselves from workspaces they don't own via the new DELETE /workspace/{workspace}/membership endpoint. The invitation flow for new users is unchanged.

  5. LOW — CSV formula injection in the activity export. Recipient email, subject, and template slug now pass through the same formulaSafe() guard the suppression export already uses.

Reviewer notes

  • Deploy note: installs running behind a reverse proxy must set TRUSTED_PROXIES to the proxy's IP/CIDR; the app no longer trusts all proxies by default.
  • Includes one new migration (add_sns_topic_arn_to_sources_table).
  • No UI calls the new leave-workspace endpoint yet — the backend fix was scoped minimal per the audit brief; a "Leave workspace" button is a possible follow-up.
  • Full test suite: 357 passed / 0 failed; Pint clean.

🤖 Generated with Claude Code

vijaythecoder and others added 8 commits October 17, 2026 13:44
Header names are now validated against RFC 5322 ftext (alphanumerics and
hyphens only) in SendEmailRequest, covering both the API and dashboard
send paths, and MimeMessageBuilder refuses to emit any custom header
whose name or value contains CR or LF as defense in depth.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The SES webhook now stores the TopicArn from the first confirmed SNS
subscription on the source and rejects any later envelope — confirmation
or notification — signed for a different topic, so AWS-signed messages
from an attacker's own SNS topic can no longer forge delivery or bounce
events. Envelopes whose Timestamp is older than 15 minutes, more than
5 minutes in the future, or unparseable are rejected as replays. All
rejections are logged to WebhookLog with status "rejected".

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bootstrap/app.php no longer calls trustProxies(at: '*'), which let any
client control $request->ip() via X-Forwarded-For. Operators behind a
reverse proxy list it in the new TRUSTED_PROXIES env variable
(comma-separated IPs/CIDRs, documented in .env.example, default none).
The Fortify login limiter now keys on the socket address (REMOTE_ADDR),
matching the existing larasend-api-ip limiter, so rotating forwarded
headers can no longer reset the per-email login throttle.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A user's active workspace now resolves to a workspace they own before
falling back to membership order (both in ProjectContext::workspaceFor
and the HandleInertiaRequests settings navigation), so being added to
someone else's lower-id workspace can no longer silently replace their
entire web context. Users can also now remove themselves from any
workspace they do not own via DELETE /workspace/{workspace}/membership.
The invitation flow for new users is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The activity export now runs recipient email, subject, and template slug
through the same formulaSafe() guard the suppression export already
uses, so attacker-controlled values can no longer execute as spreadsheet
formulas when the CSV is opened in Excel or similar tools.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
composer audit in the quality CI job flagged known CVEs in
laravel/framework, league/commonmark, league/flysystem,
symfony/http-foundation, and symfony/yaml. Updates those packages (and
their transitive dependencies) within existing composer.json
constraints; the full test suite passes against the new versions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
laravel/framework 13.37 registers the HTTP QUERY verb on redirect
routes, and Wayfinder 0.1.20 generated TypeScript for it that its own
runtime types rejected, failing vue-tsc in the quality CI job. Updates
laravel/wayfinder to 0.1.23 (whose Method type includes "query"), which
required bumping laravel/boost and its laravel/mcp and laravel/roster
dependencies. vue-tsc, composer audit, and the full test suite pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
npm audit fix plus bumping the Tiptap packages to 3.32.0 clears every
advisory in production dependencies (including the high-severity
@tiptap/core XSS/ReDoS advisories and the critical shell-quote issue).
The remaining high finding, braces GHSA-vfj7-8cjw-p6xm, is dev-only
(reached via @vue/eslint-config-typescript's lint tooling), affects all
published versions with no fix, and its forced remediation downgrades
the eslint config below the defineConfigWithVueTs API this repo uses —
so the CI audit step now runs with --omit=dev, documented inline.
Format, lint, types, build, and the full test suite all pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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