Skip to content

fix(security): resolve CSP syntax error and remove unreachable code - #990

Open
Chigael wants to merge 10 commits into
csxark:mainfrom
Chigael:fix/csp-syntax-error-unreachable-code
Open

fix(security): resolve CSP syntax error and remove unreachable code#990
Chigael wants to merge 10 commits into
csxark:mainfrom
Chigael:fix/csp-syntax-error-unreachable-code

Conversation

@Chigael

@Chigael Chigael commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

This PR fixes syntax errors and unclosed brace structures in lib/security/contentSecurityPolicy.ts that were causing TypeScript compilation failures (TS1005: '}' expected).

Related Issue

Closes #963

Type of Change

  • Bug Fix
  • New Feature
  • Documentation
  • UI/UX Improvement
  • Refactoring
  • Performance Improvement
  • Accessibility

Changes Made

  • Fixed the else brace structure so the throw new Error("CSPRNG not available") statement is properly scoped.
  • Removed the unreachable Math.random() loop (which is cryptographically insecure for CSP nonces).
  • Cleaned up trailing leftover call to globalThis.crypto.getRandomValues(randomValues).
  • spContainsUnsafeInline(): Removed duplicate, unreachable return statement with doubly-escaped regex.

Testing

  • Tested locally
  • No console errors
  • Existing functionality works as expected

Screenshots

If applicable, attach screenshots or screen recordings.


Checklist

  • My code follows the project guidelines.
  • I have tested my changes.
  • I have updated the documentation if required.
  • My changes address the related issue.
  • This PR focuses on a single issue.

Additional Notes

Add any additional information for reviewers here.

Summary by CodeRabbit

  • Bug Fixes
    • Improved Content Security Policy validation for more accurate security directive and nonce checks.
    • Corrected detection of unsafe inline content, including whitespace variations.
    • Strengthened nonce generation by requiring secure browser cryptography.

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown

@Chigael is attempting to deploy a commit to the csxark's projects Team on Vercel.

A member of the Team first needs to authorize it.

@github-actions

Copy link
Copy Markdown
Contributor

🎉 Thank You for Your Contribution

Hello @Chigael,

Thank you for submitting a Pull Request to CryptoViz. We appreciate the time and effort you've invested in contributing to the project.

Your Pull Request has been received successfully and will be reviewed by the maintainers as soon as possible.

📋 Pull Request Checklist

  • ✅ Linked the related issue
  • ✅ No merge conflicts
  • ✅ Synced with the latest main branch
  • ✅ Build passes successfully
  • ✅ All tests pass

Ensuring these requirements are met helps streamline the review process and enables maintainers to review your contribution more efficiently.

❤️ Support CryptoViz

If you find CryptoViz helpful, consider supporting the project by:

  • ⭐ Starring the repository
  • 🍴 Forking the repository
  • 👤 Following Ark on GitHub
  • 💼 Connecting on LinkedIn
  • 🌐 Visiting the Portfolio

Your support helps increase the project's visibility and encourages continued development. Thank you for being a part of the CryptoViz community!

Thank you once again for contributing to CryptoViz. We appreciate your support and look forward to reviewing your contribution.

@coderabbitai

coderabbitai Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@Chigael, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 31 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 0f289566-db03-4bf8-be95-24b4a63c191b

📥 Commits

Reviewing files that changed from the base of the PR and between ac5756f and 30851f5.

📒 Files selected for processing (2)
  • lib/security/contentSecurityPolicy.ts
  • tests/unit/security/contentSecurityPolicy.test.ts
📝 Walkthrough

Walkthrough

The CSP security module now requires Web Crypto for nonce generation, detects actual whitespace in unsafe-inline, and applies boundary-aware validation for CSP directives and nonce values.

Changes

CSP validation

Layer / File(s) Summary
CSP nonce and directive validation
lib/security/contentSecurityPolicy.ts
The CSP result type formatting is unchanged semantically. Nonce generation now requires globalThis.crypto.getRandomValues and throws when unavailable. Unsafe-inline detection matches actual whitespace. Directive validation now requires directive boundaries, whitespace, and complete values.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

  • csxark/CryptoViz#156 — This PR also changes CSP security behavior by removing 'unsafe-inline' from script-src and adding CSP regression tests.

Suggested labels: ECSoC26-L2, needs review

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: fixing CSP syntax errors and removing unreachable code.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@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

🤖 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 `@lib/security/contentSecurityPolicy.ts`:
- Around line 131-134: Update the hasScriptNonce detection in the CSP validation
logic to recognize a nonce followed by additional script-src source expressions
such as strict-dynamic. Replace the directive-end requirement after the nonce
with a token boundary and allow the remaining directive values to be consumed,
while preserving matching across directive boundaries.
🪄 Autofix

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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: f29bdfc5-19ba-45ea-9df7-66f8b6507eb5

📥 Commits

Reviewing files that changed from the base of the PR and between eddaffe and 2b5681c.

📒 Files selected for processing (3)
  • lib/offline/precacheRoutes.ts
  • lib/security/contentSecurityPolicy.ts
  • public/sw.js

Comment thread lib/security/contentSecurityPolicy.ts
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@csxark

csxark commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Hi @Chigael resolve the merge conflicts

@csxark csxark added ECSoC26 Elite Coders Summer of Code 2026 and removed needs review labels Aug 10, 2026
@Chigael

Chigael commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@csxark kindly reopen, so I can resolve conflict and then possible merge the PR

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ECSoC26 Elite Coders Summer of Code 2026

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lib/security/contentSecurityPolicy.ts has syntax error rendering CSP module inoperable

2 participants