feat(http-helmet): remove insecure response headers - #148
Conversation
Add removeInsecureHeaders to strip server-identifying headers from response Headers without mutating caller state. Seed the removal list from the OWASP Secure Headers Project and add a scheduled workflow that validates upstream input, regenerates the list deterministically, and opens a changeset-backed pull request. Apply the helper in the React Router v7 example and cover insecure-header removal, safe-header preservation, and input immutability.
Use absolute Oniguruma anchors in the updater jq validator so a trailing newline cannot satisfy the end-of-line anchor. This keeps generated removal lists restricted to complete, valid HTTP field-name tokens.
🦋 Changeset detectedLatest commit: f551753 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
packages | f551753 | Aug 02 2026, 02:05 AM |
WalkthroughThe change adds ChangesResponse header sanitization
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ReactRouterServer
participant removeInsecureHeaders
participant removalHeaders
ReactRouterServer->>removeInsecureHeaders: pass responseHeaders
removeInsecureHeaders->>removalHeaders: read configured header names
removeInsecureHeaders-->>ReactRouterServer: return sanitized headers
ReactRouterServer->>ReactRouterServer: merge secure headers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #148 +/- ##
==========================================
+ Coverage 93.97% 94.56% +0.58%
==========================================
Files 19 20 +1
Lines 896 993 +97
Branches 221 222 +1
==========================================
+ Hits 842 939 +97
Misses 54 54 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
commit: |
There was a problem hiding this comment.
🟡 Not ready to approve
The updater workflow’s validation currently allows case-insensitive duplicate headers, and the new public utility’s exported signature should be made more robust/explicit for API stability.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR adds an opt-in removeInsecureHeaders utility to @mcansh/http-helmet that strips a curated list of server/framework/diagnostic response headers (seeded from OWASP Secure Headers), updates an example app to apply the removal before merging security headers, and introduces a scheduled workflow to keep the bundled removal list refreshed via automated PRs.
Changes:
- Added
removeInsecureHeadersand re-exported it from the package entrypoint. - Introduced a generated
removalHeaderslist (OWASP-seeded) plus Vitest coverage verifying removal + non-mutation. - Added a GitHub Actions workflow to validate upstream JSON and regenerate
removal-headers.tswith a changeset-backed PR.
File summaries
| File | Description |
|---|---|
| packages/http-helmet/src/utils.ts | Adds removeInsecureHeaders implementation and pulls in the removal list. |
| packages/http-helmet/src/utils.spec.ts | Adds tests for removal coverage, preservation of unrelated headers, and non-mutation. |
| packages/http-helmet/src/removal-headers.ts | Adds the OWASP-seeded removal header name list. |
| packages/http-helmet/src/index.ts | Re-exports removeInsecureHeaders from the public package entrypoint. |
| apps/http-helmet/react-router-v7/app/entry.server.tsx | Applies removeInsecureHeaders before merging generated secure headers in the example. |
| .github/workflows/update-removal-headers.yml | Adds scheduled/manual updater workflow that validates and regenerates the removal list and opens/updates a PR. |
| .changeset/tidy-lions-protect.md | Adds a patch changeset documenting the new public utility. |
Review details
- Files reviewed: 7/7 changed files
- Comments generated: 2
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| export function removeInsecureHeaders(responseHeaders: Headers) { | ||
| const headers = new Headers(responseHeaders); | ||
|
|
||
| for (const key of removalHeaders) { | ||
| headers.delete(key); | ||
| } | ||
|
|
||
| return headers; | ||
| } |
| jq --exit-status ' | ||
| .headers | ||
| | type == "array" | ||
| and length > 0 | ||
| and length <= 1000 | ||
| and (unique | length) == length | ||
| and all(.[]; | ||
| if type == "string" then | ||
| length > 0 | ||
| and length <= 256 | ||
| and test("\\A[!#$%&\u0027*+.^_`|~0-9A-Za-z-]+\\z") | ||
| else | ||
| false | ||
| end | ||
| ) | ||
| ' "$response_file" > /dev/null |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/update-removal-headers.yml:
- Around line 22-26: Update the checkout step using actions/checkout to set
persist-credentials to false, then add explicit Git authentication with gh auth
setup-git before the later bare git push commands in the script, using the
existing GH_TOKEN environment configuration.
- Around line 8-10: Move the workflow-level permissions block into the
jobs.update job so contents: write and pull-requests: write apply only to that
job. Add brief comments documenting why each permission is required, preserving
the existing permission scopes and workflow behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f24e3800-61e5-4546-bcc7-f51d2bd54297
📒 Files selected for processing (7)
.changeset/tidy-lions-protect.md.github/workflows/update-removal-headers.ymlapps/http-helmet/react-router-v7/app/entry.server.tsxpackages/http-helmet/src/index.tspackages/http-helmet/src/removal-headers.tspackages/http-helmet/src/utils.spec.tspackages/http-helmet/src/utils.ts
| permissions: | ||
| contents: write | ||
| pull-requests: write |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Scope permissions to the job, not the workflow.
contents: write and pull-requests: write are declared at the workflow level. This grants both permissions to every job in the workflow, including any jobs added later, rather than only the update job that needs them. Move permissions under jobs.update and add a short comment on why each scope is needed.
🔒 Proposed fix
-permissions:
- contents: write
- pull-requests: write
+permissions:
+ contents: read
concurrency:
group: update-http-helmet-removal-headers
cancel-in-progress: false
jobs:
update:
name: 🛡️ Update removal headers
runs-on: ubuntu-latest
timeout-minutes: 10
+ permissions:
+ contents: write # push the automation branch with the regenerated removal-header list
+ pull-requests: write # open or update the changeset-backed pull request
steps:As per the static analysis hints, this triggers excessive-permissions at lines 9-10 and undocumented-permissions at line 9.
🧰 Tools
🪛 zizmor (1.28.0)
[error] 9-9: overly broad permissions (excessive-permissions): contents: write is overly broad at the workflow level
(excessive-permissions)
[error] 10-10: overly broad permissions (excessive-permissions): pull-requests: write is overly broad at the workflow level
(excessive-permissions)
[warning] 9-9: permissions without explanatory comments (undocumented-permissions): needs an explanatory comment
(undocumented-permissions)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/update-removal-headers.yml around lines 8 - 10, Move the
workflow-level permissions block into the jobs.update job so contents: write and
pull-requests: write apply only to that job. Add brief comments documenting why
each permission is required, preserving the existing permission scopes and
workflow behavior.
Source: Linters/SAST tools
| - name: ⬇️ Checkout repo | ||
| uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 | ||
| with: | ||
| fetch-depth: 0 | ||
| ref: ${{ github.event.repository.default_branch }} |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Disable credential persistence and configure git auth explicitly for the push.
actions/checkout is not configured with persist-credentials: false, so the checkout token stays available in the job's git configuration for the rest of the run. The script later relies on this persisted credential for the bare git push commands (lines 118-126), since no explicit git authentication is set up otherwise. Set persist-credentials: false on checkout and configure git authentication explicitly via gh auth setup-git, which uses the GH_TOKEN already exported for the script step.
🔒 Proposed fix
- name: ⬇️ Checkout repo
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0
+ persist-credentials: false
ref: ${{ github.event.repository.default_branch }} run: |
set -euo pipefail
+ gh auth setup-git
+
response_file=$(mktemp)As per the static analysis hints, this triggers artipacked (credential persistence through GitHub Actions checkout) at lines 22-26.
🧰 Tools
🪛 zizmor (1.28.0)
[warning] 22-26: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/update-removal-headers.yml around lines 22 - 26, Update
the checkout step using actions/checkout to set persist-credentials to false,
then add explicit Git authentication with gh auth setup-git before the later
bare git push commands in the script, using the existing GH_TOKEN environment
configuration.
Source: Linters/SAST tools
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ No critical issues — minor suggestions only. The utility, tests, and workflow validation are all sound (I verified the jq token regex rejects control characters and duplicates, the OWASP list matches upstream exactly, and the package tests, typecheck, prettier, and build all pass). Two design observations and a few nitpicks below.
Reviewed changes
removeInsecureHeadersutility — new opt-in export that copies the incomingHeadersand deletes every header in the OWASP-derivedremovalHeaderslist, leaving the original object untouched.removalHeaderslist — 87 entries seeded from the OWASP Secure Headers Projectci/headers_remove.json; the committed list matches the current upstream source exactly.- Updater workflow — new
.github/workflows/update-removal-headers.ymlthat validates upstream JSON shape (count limits, uniqueness, RFC 7230 token regex with\A…\zanchors), deterministically regeneratesremoval-headers.tsviajq tojson, and opens a changeset-backed PR. - Tests —
utils.spec.tscovers every-listed-header removal, preservation of unrelated headers, and non-mutation of the original; the full package suite,tsc, prettier, and the tsdown build pass. - Example app — React Router v7
entry.server.tsxnow appliesremoveInsecureHeaders(responseHeaders)before merging secure headers. - Changeset — patch changeset for
@mcansh/http-helmet.
ℹ️ Update PRs created by this workflow won't run CI checks
The workflow opens/updates its PR using ${{ github.token }} via gh pr create. PRs created by a workflow running on GITHUB_TOKEN do not run pull_request-triggered checks automatically — GitHub places the runs in an approval-required state, so test-pr.yml and autofix.yml won't run unless a maintainer manually approves them. The generated update PR therefore merges without tests, lint, or typecheck verification.
Technical details
# Update PRs skip CI
## Affected sites
- .github/workflows/update-removal-headers.yml:148 — `gh pr create` under the workflow's `github.token`
- .github/workflows/update-removal-headers.yml:128 — `gh pr list` update path, same token
## Required outcome
- Decide and document how generated update PRs get verified. Options: (a) accept the tradeoff — input is strictly schema/token validated, output is generated deterministically, and a human reviews before merge; (b) approve the approval-required `pull_request` runs when reviewing; or (c) use a GitHub App / PAT token for the push + PR steps so `pull_request` workflows fire normally.
## Open questions for the human
- Is the lack of automated checks on generated PRs acceptable given the strict upstream validation?ℹ️ The removal list has no per-header escape hatch
removeInsecureHeaders always removes every entry in the OWASP list. Several of those entries are headers real applications depend on — SourceMap/X-SourceMap (production source maps for error reporting), X-B3-* (Zipkin), X-Datadog-*, X-Envoy-* (mesh observability). A consumer who opts in but relies on any of these has no way to keep them. Consider documenting this explicitly, or in a future iteration accept an override set (e.g. removeInsecureHeaders(headers, { exclude: ["SourceMap"] })).
ℹ️ Nitpicks
removeInsecureHeadersis a new public API but the package README doesn't mention it — the README documentsmergeHeaders/createNonceusage but has no example for the new utility.- The changeset bumps
@mcansh/http-helmetaspatch, but this adds a new public export (the PR title isfeat(...)); conventional changesets practice would beminorfor an additive feature.
Big Pickle (free) ｜ 𝕏
There was a problem hiding this comment.
✅ Pullfrog addressed all of its review feedback on this PR.
Addressed review #4834820278 (0 inline threads; feedback was in the review body).
Applied
- Added a "Remove Insecure Headers" section to
packages/http-helmet/README.mddocumentingremoveInsecureHeadersusage. The docs also state there's no per-header allowlist and that every OWASP-listed header is removed — addressing the "no escape hatch" observation's "document this explicitly" suggestion without adding an override API (avoiding scope creep for a first version). Committed and pushed asf551753.
Not applied, with reasons
- Changeset
patch→minor: keptpatch. This repo's own changelog precedent labels additive new exports as patch changes (e.g. 0.13.0 "add prequoted keyword exports"), reservingminorfor potentially-breaking changes (peerDep range updates, behavior changes). Installed@changesets/cli@2.29.7uses standard semver increments, sominorwould overstate an opt-in, purely additive export. - CI on generated update PRs (approval-required runs from
github.token-created PRs): left as an author decision — options (a) accept the tradeoff given strict schema/token validation, (b) approve the runs manually, or (c) switch to a GitHub App/PAT. I can't configure (c) from here, and (a)/(b) are policy choices.
Verification: http-helmet suite (28 tests) passes; prettier clean.
Task list (6/6 completed)
- Checkout PR 148 and read review threads (0 threads, review body has 2 nitpicks + 2 design observations)
- Add removeInsecureHeaders documentation to package README
- Evaluate changeset patch vs minor: keep patch (repo precedent = additive exports are patch), document reasoning
- Evaluate 2 design observations (workflow CI tradeoff, escape hatch) - escape hatch documented in README, CI tradeoff left for author
- Verify changes (tests + prettier), commit, push
- Finalize: report_progress summary
Big Pickle (free) ｜ 𝕏
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/http-helmet/README.md`:
- Around line 77-80: Update the README example around removeInsecureHeaders to
define responseHeaders before use, preferably by initializing it as a Headers
instance, so the snippet is self-contained and runnable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d92b126-6727-448d-b12b-62cc7ec79b5b
📒 Files selected for processing (1)
packages/http-helmet/README.md
| ```js | ||
| import { removeInsecureHeaders } from "@mcansh/http-helmet"; | ||
|
|
||
| let headers = removeInsecureHeaders(responseHeaders); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Define responseHeaders in the example.
The snippet uses responseHeaders without declaring it. Add a Headers instance or show the call within a complete response-handling example so users can run the documented code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/http-helmet/README.md` around lines 77 - 80, Update the README
example around removeInsecureHeaders to define responseHeaders before use,
preferably by initializing it as a Headers instance, so the snippet is
self-contained and runnable.
Summary
Adds an opt-in
removeInsecureHeadersutility that removes server-identifying, framework, and diagnostic response headers maintained by the OWASP Secure Headers Project without mutating the originalHeadersobject.What Changed
removeInsecureHeadersfrom@mcansh/http-helmet.@mcansh/http-helmet.Test Coverage Updates
Headersobject unchanged.Impact
Risk And Rollback
Set-Cookievalues remain preserved.Summary by CodeRabbit
removeInsecureHeadersto sanitize response headers by removing server, framework, proxy, and diagnostic identifiers.