Security hardening: demo fix, overflow guard, SECURITY.md, CodeQL, CI pinning#16
Conversation
…r in light mode xterm.css hardcodes background-color: #000 on .xterm-viewport and never re-syncs it to the theme passed into the Terminal constructor. The terminal's text rows only cover a whole number of cell rows, so the strip between the last row and the container's bottom edge showed that un-themed black viewport through as a black bar in light mode. #terminal .xterm-viewport already themed the scrollbar; it now also sets background-color: var(--bg) so the viewport follows the page theme in both modes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uffers EncryptInternal computed byteLen = plain.Length * 2 unchecked, while the build-buffer growth path a few hundred lines up already wraps the identical doubling in checked(...). A plaintext at or beyond int.MaxValue / 2 UTF-16 chars would overflow byteLen to a negative or too-small value, under-allocating the buffer the encryption then writes into. Wrapped the multiplication in checked(...) to match the existing convention, so construction now throws OverflowException instead of risking a truncated allocation. README: documented the new threshold and behavior in the Memory footprint section. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ege hardening - Add SECURITY.md: supported-versions policy, private-vulnerability- reporting instructions (already referenced from CONTRIBUTING.md but never actually written down), response expectations, and an explicit scope section deferring to the README's documented limitations. - Add codeql.yml: CodeQL analysis for the C# surface on push/PR to master plus a weekly schedule, scoped to net10.0 on ubuntu-latest via the same CI_TFMS mechanism ci.yml's ubuntu leg already uses (no extra workloads needed). - ci.yml and build-platform-tfms.yml previously had no explicit permissions: block, so they ran under whatever the repo/org default GITHUB_TOKEN scope happened to be. Both only build/test, so they now explicitly declare contents: read. - Pin every third-party Action (actions/checkout, actions/setup-dotnet, actions/upload-pages-artifact, actions/deploy-pages, dependabot/fetch-metadata, googleapis/release-please-action) to the commit SHA the currently-pinned tag resolves to, with a trailing "# vN" comment for readability. Tags are mutable; a compromised action maintainer could re-point one without Dependabot noticing until the next real release. Behavior is unchanged - same version, just an immutable reference. Dependabot already tracks the github-actions ecosystem, so it can still propose SHA bumps going forward. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 49 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the 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 configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR pins GitHub Actions to commit SHAs and adds explicit permissions across build, CI, dependabot, pages, and release workflows; adds a new CodeQL analysis workflow; adds SECURITY.md; fixes an integer overflow guard in ProtectedString's encryption length calculation with matching README documentation; and adjusts demo Wasm terminal CSS. ChangesCI/CD workflow hardening and CodeQL
Security policy documentation
Plaintext length overflow guard and demo UI fix
Estimated code review effort: 2 (Simple) | ~12 minutes Sequence Diagram(s)Not applicable — changes are configuration pinning, documentation, and a small logic guard without multi-component interaction flows. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/codeql.yml:
- Around line 37-38: The Checkout step in the code scanning workflow is leaving
credentials persisted by default. Update the actions/checkout usage in the
workflow to explicitly disable credential persistence with persist-credentials
set to false, keeping the hardening consistent with the rest of the job; use the
Checkout step in codeql.yml as the place to make this change.
- Around line 25-27: The workflow-level permissions are too broad and should be
narrowed to the job that actually needs them. Move the `security-events: write`
permission from the top of the CodeQL workflow into the single job that uploads
results, and add a brief explanatory comment there so the least-privilege intent
is explicit. Use the existing workflow job definition in
`.github/workflows/codeql.yml` to scope `permissions` at the job level.
- Line 44: The workflow still pins github/codeql-action to the old major
version, so update both CodeQL action references in the CodeQL workflow from v3
to v4. Adjust the action versions used by the init and analyze steps so the
workflow consistently targets the supported v4 major line, keeping the existing
setup and step structure unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1a0875d6-ec5c-4b38-a5ab-344f654526a4
📒 Files selected for processing (10)
.github/workflows/build-platform-tfms.yml.github/workflows/ci.yml.github/workflows/codeql.yml.github/workflows/dependabot-auto-merge.yml.github/workflows/pages.yml.github/workflows/release.ymlREADME.mdSECURITY.mdTopSecret.Demo.Wasm/wwwroot/index.htmlTopSecret.ProtectedString/ProtectedString.cs
- Set persist-credentials: false on the checkout step — this job never needs to push, so there's no reason to leave the token in the local git config for the rest of the job. - Scope security-events: write (and contents: read) to the analyze job itself instead of the workflow-level default. Note: job-level permissions fully replace the workflow-level block rather than merging with it, so contents: read had to move down too, not just the security-events grant, or the job would have silently lost read access. - Bump github/codeql-action from v3 to v4 (the current major per upstream's own README) on both the init and analyze steps, pinned to the new v4 tag's resolved commit SHA. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2 participants
Summary
EncryptInternal's ciphertext-length calculation against 32-bit overflow withchecked(...), matching the existing convention already used for the build-buffer growth path; documented the new threshold in the README.SECURITY.md(supported versions, private vulnerability reporting, scope).permissions: contents: readtoci.ymlandbuild-platform-tfms.yml.Test plan
ci.ymland the newcodeql.ymlboth go green on this PR before merging.checked(...)pattern already in the same file.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores