Skip to content

fix: address critical security vulnerabilities#1

Open
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775932637-security-fixes
Open

fix: address critical security vulnerabilities#1
devin-ai-integration[bot] wants to merge 1 commit intomainfrom
devin/1775932637-security-fixes

Conversation

@devin-ai-integration
Copy link
Copy Markdown

Why:

Security audit of the codebase identified several issues: exposed debug endpoints accessible outside development, unvalidated user-controlled input used in HTTP responses, and known-vulnerable transitive dependencies.

What's being changed (if available, include any code snippets, screenshots, or gifs):

Three targeted code fixes + dependency patching:

  1. src/frame/middleware/fastly-cache-test.ts — The X-CacheTest-Error header value was passed directly to res.status(parseInt(...)) with no validation. Malformed values (NaN, negative, >599) could cause unexpected Express behavior. Now validates the parsed integer is a valid HTTP status code (100–599) and returns 400 otherwise.

  2. src/frame/middleware/req-headers.ts — The /_req-headers endpoint unconditionally returned all request headers (including cookies, auth tokens, internal proxy headers) as JSON in every environment. Now gated to NODE_ENV === 'development' only; returns 404 elsewhere.

  3. src/observability/middleware/trigger-error.ts — The /_500 endpoint was guarded by NODE_ENV === 'production' && MODA_PROD_SERVICE_ENV === 'true', meaning it remained accessible in staging. Changed to an allowlist: only fires in development or test environments.

  4. package-lock.jsonnpm audit fix patched vulnerable transitive dependencies including ajv (ReDoS), vite (path traversal / arbitrary file read), and proxy-from-env.

⚠️ Reviewers should verify:

  • The trigger-error.ts allowlist change: confirm no non-dev/test environment legitimately needs this endpoint for error-handling validation.
  • The req-headers.ts route is still registered in the middleware stack but returns 404 in non-dev. Confirm this is acceptable vs. removing the route entirely.
  • package-lock.json changes are from npm audit fix — transitive-only updates, but worth a sanity check.

Check off the following:

  • A subject matter expert (SME) has reviewed the technical accuracy of the content in this PR. In most cases, the author can be the SME. Open source contributions may require an SME review from GitHub staff.
  • The changes in this PR meet the docs fundamentals that are required for all content.
  • All CI checks are passing and the changes look good in the review environment.

Link to Devin session: https://app.devin.ai/sessions/30070cf782bd4b4aae917569b573e6a7
Requested by: @lalo458

- Validate X-CacheTest-Error header value in fastly-cache-test.ts to
  prevent arbitrary HTTP status codes (must be 100-599)
- Restrict /_req-headers endpoint to development environment only to
  prevent leaking sensitive request headers in production/staging
- Tighten /_500 trigger-error guard to only allow in development and
  test environments, blocking staging deployments
- Run npm audit fix to patch vulnerable dependencies (axios SSRF,
  liquidjs file read, lodash prototype pollution, etc.)

Co-Authored-By: laloesparza458 <laloesparza458@gmail.com>
@devin-ai-integration
Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

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