Repository navigation
fix(http-helmet): encode nonces across JavaScript runtimes - #157
Conversation
🦋 Changeset detectedLatest commit: cce6c4d 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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. Walkthrough
ChangesNonce encoding
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The nonce encoding change appears mergeable after normal checks. Other JavaScript runtimes remain untested, but no specific failure has been identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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 |
commit: |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #157 +/- ##
==========================================
+ Coverage 93.97% 94.23% +0.26%
==========================================
Files 19 19
Lines 896 902 +6
Branches 221 223 +2
==========================================
+ Hits 842 850 +8
+ Misses 54 52 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The implementation preserves existing output, handles both runtime paths, and includes focused tests and release metadata.
Review effort: Balanced
Findings: None
What changed in this PR
Makes createNonce() portable across JavaScript runtimes while preserving its Base64-encoded UUID format.
Changes:
- Uses native
Uint8Array.toBase64()with abtoa()fallback. - Tests native and fallback paths.
- Adds a patch Changeset.
| File | Description |
|---|---|
packages/http-helmet/src/utils.ts |
Implements portable nonce encoding. |
packages/http-helmet/src/nonce.spec.ts |
Covers encoding paths and allocation behavior. |
.changeset/portable-http-helmet-nonce.md |
Records the patch release. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ❌ Deployment failed View logs |
packages | cce6c4d | Oct 07 2026, 02:32 PM |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
createNonceBuffer removal (packages/http-helmet/src/utils.ts) —Buffer.from(uuid).toString("base64")replaced withUint8Array.prototype.toBase64when present and abtoa(uuid)fallback otherwise;toBase64is read once, guarded by atypeofcheck, and invoked with a freshTextEncoderbyte buffer.- Nonce tests (
packages/http-helmet/src/nonce.spec.ts) — a distinctness + base64-decodable-UUID-format test, and a parameterized test that stubsUint8Array.prototype.toBase64presence/absence, spiesTextEncoder.prototype.encodeandcrypto.randomUUID, and asserts both branches returnbtoa(uuid), the native branch callsencodeexactly once, and the fallback skips UTF-8 allocation. - Changeset —
patchfor@mcansh/http-helmet.
I verified the equivalence claim directly: for an ASCII UUID, new TextEncoder().encode(uuid).toBase64() and btoa(uuid) both equal Buffer.from(uuid).toString("base64"), so the output format is unchanged on every path. The parameterized tests genuinely pin the branching (always-native or always-fallback implementations would fail the assertions), and the finally block restores the stubbed prototype cleanly. Full package suite (28 tests), tsc --noEmit, and Prettier all pass locally.
big-pickle (free) ｜ 𝕏
Summary
createNonce()currently depends on the Node.jsBufferglobal. Use native Base64 encoding when available and a portable fallback otherwise, preserving the Base64-encoded UUID format.Changes include tests for both paths, a check that the fallback skips UTF-8 byte allocation, and a patch changeset for
@mcansh/http-helmet.Related stack: #150 → #151 → #152 → #153. This PR targets
devdirectly, so it is independent of that native stack.Impact: Only nonce generation changes; consumers retain the existing string format and need no configuration changes.
Evidence
Buffer.from(uuid).toString("base64").Merge Danger
Door: two-way. Revert this commit to restore the previous implementation; no data migration is involved.
Blast Radius: nonces. Runtime differences in native Base64 encoding could affect CSP nonce strings; tests verify equivalent output and fallback behavior.
Summary by CodeRabbit