Skip to content

@apostrophecms/security-headers module setup. - #225

Merged
vasilyyaremchuk merged 1 commit into
mainfrom
feature/security-headers
Sep 9, 2025
Merged

@apostrophecms/security-headers module setup.#225
vasilyyaremchuk merged 1 commit into
mainfrom
feature/security-headers

Conversation

@vasilyyaremchuk

Copy link
Copy Markdown
Collaborator

No description provided.

@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2025

Copy link
Copy Markdown

@github-actions

github-actions Bot commented Sep 9, 2025

Copy link
Copy Markdown

🔍 Vulnerabilities of apostrophe-cms:test

📦 Image Reference apostrophe-cms:test
digestsha256:888537f6840e7859fd9be5a3f1831979f85a273a0dd14db41f57a1152d160a54
vulnerabilitiescritical: 1 high: 4 medium: 0 low: 0
platformlinux/amd64
size291 MB
packages985
📦 Base Image node:23-alpine
also known as
  • 23-alpine3.22
  • 23.11-alpine
  • 23.11-alpine3.22
  • 23.11.1-alpine
  • 23.11.1-alpine3.22
digestsha256:b9d38d589853406ff0d4364f21969840c3e0397087643aef8eede40edbb6c7cd
vulnerabilitiescritical: 0 high: 0 medium: 1 low: 3
critical: 1 high: 0 medium: 0 low: 0 form-data 4.0.2 (npm)

pkg:npm/form-data@4.0.2

critical 9.4: CVE--2025--7783 Use of Insufficiently Random Values

Affected range>=4.0.0
<4.0.4
Fixed version4.0.4
CVSS Score9.4
CVSS VectorCVSS:4.0/AV:N/AC:H/AT:N/PR:N/UI:N/VC:H/VI:H/VA:N/SC:H/SI:H/SA:N
EPSS Score0.024%
EPSS Percentile5th percentile
Description

Summary

form-data uses Math.random() to select a boundary value for multipart form-encoded data. This can lead to a security issue if an attacker:

  1. can observe other values produced by Math.random in the target application, and
  2. can control one field of a request made using form-data

Because the values of Math.random() are pseudo-random and predictable (see: https://blog.securityevaluators.com/hacking-the-javascript-lottery-80cc437e3b7f), an attacker who can observe a few sequential values can determine the state of the PRNG and predict future values, includes those used to generate form-data's boundary value. The allows the attacker to craft a value that contains a boundary value, allowing them to inject additional parameters into the request.

This is largely the same vulnerability as was recently found in undici by parrot409 -- I'm not affiliated with that researcher but want to give credit where credit is due! My PoC is largely based on their work.

Details

The culprit is this line here: https://github.com/form-data/form-data/blob/426ba9ac440f95d1998dac9a5cd8d738043b048f/lib/form_data.js#L347

An attacker who is able to predict the output of Math.random() can predict this boundary value, and craft a payload that contains the boundary value, followed by another, fully attacker-controlled field. This is roughly equivalent to any sort of improper escaping vulnerability, with the caveat that the attacker must find a way to observe other Math.random() values generated by the application to solve for the state of the PRNG. However, Math.random() is used in all sorts of places that might be visible to an attacker (including by form-data itself, if the attacker can arrange for the vulnerable application to make a request to an attacker-controlled server using form-data, such as a user-controlled webhook -- the attacker could observe the boundary values from those requests to observe the Math.random() outputs). A common example would be a x-request-id header added by the server. These sorts of headers are often used for distributed tracing, to correlate errors across the frontend and backend. Math.random() is a fine place to get these sorts of IDs (in fact, opentelemetry uses Math.random for this purpose)

PoC

PoC here: https://github.com/benweissmann/CVE-2025-7783-poc

Instructions are in that repo. It's based on the PoC from https://hackerone.com/reports/2913312 but simplified somewhat; the vulnerable application has a more direct side-channel from which to observe Math.random() values (a separate endpoint that happens to include a randomly-generated request ID).

Impact

For an application to be vulnerable, it must:

  • Use form-data to send data including user-controlled data to some other system. The attacker must be able to do something malicious by adding extra parameters (that were not intended to be user-controlled) to this request. Depending on the target system's handling of repeated parameters, the attacker might be able to overwrite values in addition to appending values (some multipart form handlers deal with repeats by overwriting values instead of representing them as an array)
  • Reveal values of Math.random(). It's easiest if the attacker can observe multiple sequential values, but more complex math could recover the PRNG state to some degree of confidence with non-sequential values.

If an application is vulnerable, this allows an attacker to make arbitrary requests to internal systems.

critical: 0 high: 1 medium: 0 low: 0 connect-multiparty 2.2.0 (npm)

pkg:npm/connect-multiparty@2.2.0

high 7.8: CVE--2022--29623 Unrestricted Upload of File with Dangerous Type

Affected range<=2.2.0
Fixed versionNot Fixed
CVSS Score7.8
CVSS VectorCVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
EPSS Score0.320%
EPSS Percentile54th percentile
Description

An arbitrary file upload vulnerability in the file upload module of Express Connect-Multiparty 2.2.0 allows attackers to execute arbitrary code via a crafted PDF file. NOTE: the Supplier has not verified this vulnerability report.

critical: 0 high: 1 medium: 0 low: 0 async 1.5.2 (npm)

pkg:npm/async@1.5.2

high 7.8: CVE--2021--43138 OWASP Top Ten 2017 Category A9 - Using Components with Known Vulnerabilities

Affected range<2.6.4
Fixed version2.6.4, 3.2.2
CVSS Score7.8
CVSS VectorCVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
EPSS Score0.907%
EPSS Percentile75th percentile
Description

A vulnerability exists in Async through 3.2.1 (fixed in 3.2.2), which could let a malicious user obtain privileges via the mapValues() method.

critical: 0 high: 1 medium: 0 low: 0 linkifyjs 4.2.0 (npm)

pkg:npm/linkifyjs@4.2.0

high 8.8: CVE--2025--8101 Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution')

Affected range<4.3.2
Fixed version4.3.2
CVSS Score8.8
CVSS VectorCVSS:4.0/AV:N/AC:L/AT:N/PR:N/UI:N/VC:L/VI:H/VA:L/SC:N/SI:N/SA:N
EPSS Score0.065%
EPSS Percentile21st percentile
Description

Improperly Controlled Modification of Object Prototype Attributes ('Prototype Pollution') vulnerability in Linkify (linkifyjs) allows XSS Targeting HTML Attributes and Manipulating User-Controlled Variables.This issue affects Linkify: from 4.3.1 before 4.3.2.

critical: 0 high: 1 medium: 0 low: 0 async 0.9.2 (npm)

pkg:npm/async@0.9.2

high 7.8: CVE--2021--43138 OWASP Top Ten 2017 Category A9 - Using Components with Known Vulnerabilities

Affected range<2.6.4
Fixed version2.6.4, 3.2.2
CVSS Score7.8
CVSS VectorCVSS:3.1/AV:L/AC:L/PR:N/UI:R/S:U/C:H/I:H/A:H
EPSS Score0.907%
EPSS Percentile75th percentile
Description

A vulnerability exists in Async through 3.2.1 (fixed in 3.2.2), which could let a malicious user obtain privileges via the mapValues() method.

@vasilyyaremchuk
vasilyyaremchuk merged commit 22815b5 into main Sep 9, 2025
11 of 12 checks passed
@vasilyyaremchuk
vasilyyaremchuk deleted the feature/security-headers branch September 9, 2025 14:13
@coderabbitai

coderabbitai Bot commented Sep 9, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

  • In website/app.js, a new module entry '@apostrophecms/security-headers': {} is added to the modules object returned by createAposConfig. No other logic or signatures are changed.
  • In website/package.json, a new dependency "@apostrophecms/security-headers": "^1.0.2" is added under dependencies. No other modifications are made.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Pre-merge checks (1 passed, 2 warnings)

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The pull request lacks a description entirely, so there is no contextual information explaining the added module configuration or dependency changes. Please add a pull request description that briefly summarizes the changes made, highlighting the new @apostrophecms/security-headers module entry in the configuration and the corresponding dependency update in package.json.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title succinctly and accurately describes the primary change—setting up the @apostrophecms/security-headers module—making it clear to reviewers what this pull request implements.

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/security-headers

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
website/app.js (1)

13-15: Enhance CSP and HSTS Configuration
The @apostrophecms/security-headers module applies strict defaults—
• Strict-Transport-Security: max-age=31536000
• X-Frame-Options: SAMEORIGIN
• X-Content-Type-Options: nosniff
• Referrer-Policy: same-origin
• Permissions-Policy: disabled
• Content-Security-Policy: computed from default policy groups (general, googleFonts, oembed, analytics etc.) with a per-request nonce available as {{ nonce }} in templates (github.com).

Override any header via module options (e.g., override options.policies for CSP or options.legitimateHosts; set Content-Security-Policy to false or a custom string) (github.com). To run CSP in report-only mode, add a Content-Security-Policy-Report-Only option with your policy string.

Before rolling this out to production:

  • Deploy CSP in report-only to monitor violations.
  • Whitelist expected origins (e.g., GTM/GA via the SEO module, your domain/CDN/upload hosts, YouTube/Vimeo embeds).
  • Enable HSTS only on HTTPS production.
  • Smoke-test the Apostrophe admin UI, preview routes, and any embedded widgets.

Let me know if you’d like a tailored CSP draft.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2c4e1f4 and fcfb670.

⛔ Files ignored due to path filters (1)
  • website/package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (2)
  • website/app.js (1 hunks)
  • website/package.json (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: lint
  • GitHub Check: e2e-tests
  • GitHub Check: security-scan
  • GitHub Check: unit-tests
🔇 Additional comments (1)
website/app.js (1)

13-15: LGTM: module registration is correct.

Comment thread website/package.json
Comment on lines +43 to 44
"@apostrophecms/security-headers": "^1.0.2",
"@barba/core": "^2.10.3",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Add looks good; verify version and security posture.

Before merge, confirm @apostrophecms/security-headers@^1.0.2 is the latest compatible release with your Apostrophe/Node stack and check for advisories. Also ensure the lockfile is updated in this PR to avoid drift in CI.

Run:


🏁 Script executed:

#!/bin/bash
# Check latest, peer deps, and advisories
npm view @apostrophecms/security-headers version engines peerDependencies
npm info apostrophe version
node -v
npm audit --omit=dev

Length of output: 369


🏁 Script executed:

#!/bin/bash
set -e

echo "PeerDependencies of @apostrophecms/security-headers@1.0.2:"
npm view @apostrophecms/security-headers@1.0.2 peerDependencies

echo ""
echo "Lockfiles found in repo:"
find . -type f \( -name 'package-lock.json' -o -name 'yarn.lock' \)

echo ""
echo "Detecting lockfile changes relative to base branch:"
BASE=$(git merge-base HEAD main 2>/dev/null || git merge-base HEAD origin/main 2>/dev/null || echo HEAD)
echo "Base commit: $BASE"
git diff --name-only $BASE...HEAD | grep -E 'package-lock.json|yarn.lock' || echo "No lockfile changes detected"

Length of output: 277


🏁 Script executed:

#!/bin/bash
set -e

echo "Checking security advisories in website directory:"
cd website
npm audit --omit=dev || true

echo ""
echo "Engines field for @apostrophecms/security-headers@1.0.2:"
npm view @apostrophecms/security-headers@1.0.2 engines

Length of output: 1619


security-headers upgrade and lockfile verified
@apostrophecms/security-headers@1.0.2 is the latest published version, exposes no peerDependencies or engine constraints (Node v24.3.0 is compatible) and your PR includes the updated website/package-lock.json.
npm audit --omit=dev reports 8 vulnerabilities (1 critical in form-data, 5 high in connect-multiparty, linkifyjs, on-headers, plus 2 low); run npm audit fix or otherwise resolve these before merging.

🤖 Prompt for AI Agents
In website/package.json around lines 43 to 44, npm audit (prod) reports 8
vulnerabilities after upgrading @apostrophecms/security-headers; run npm audit
--omit=dev to confirm, then run npm audit fix --omit=dev to auto-resolve what
can be fixed, manually update or replace any remaining vulnerable transitive
packages (notably form-data and connect-multiparty) by bumping direct dependency
versions or switching to maintained alternatives, regenerate and commit
website/package-lock.json, re-run the app’s test/build to verify no regressions,
and include the updated lockfile in the PR before merging.

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