Skip to content

ci: gate pushes with a11y-smoke and resolve the deferred code-scanning findings - #631

Merged
therealbrad merged 2 commits into
mainfrom
fix/post-1.0-hardening
Sep 9, 2026
Merged

ci: gate pushes with a11y-smoke and resolve the deferred code-scanning findings#631
therealbrad merged 2 commits into
mainfrom
fix/post-1.0-hardening

Conversation

@therealbrad

Copy link
Copy Markdown
Contributor

Description

Post-1.0 hardening deferred from the v1.0.0 release: the two CI gaps and the deferred code-scanning findings recorded in #605.

CI

  • a11y-smoke.yml now also runs on push to main and release/** (with its own path in both filters), so a critical violation can no longer sit on a release branch unscanned between pull requests. beta is not in the list because it was retired in ci(release): drop the retired beta branch from every workflow trigger #628.
  • All eight workflows that had no top-level permissions: block now declare contents: read. CodeQL had only flagged the three whose jobs carry no block at all (a11y-smoke, db-integration, e2e-smoke); the five release workflows already grant write per job, so there this documents the existing guarantee with no behavior change.

Code scanning

  • OfficeDocumentPreview.tsx (incomplete URL scheme check): confirmed as a real gap. The href of a link in a rendered Word document stays in the DOM with target="_blank", and only a literal javascript: prefix was stripped. Links now go through the URL parser and keep their href only for http:, https:, mailto: and tel:, which also covers data:, vbscript: and tab-obfuscated schemes.
  • Polynomial ReDoS (anthropic.adapter.ts, pricing.ts x2): /\/+$/ backtracks quadratically on long runs of /. The three flagged sites now use a new stripTrailingSlashes loop helper in lib/utils/url.ts. Identical regexes elsewhere were not flagged and are unchanged.
  • Externally-controlled format string (forecastService.ts, testRunService.ts): triaged as benign. Each site is a multi-argument console.error whose first argument interpolates an id, so a %s in the id could only garble a log line. All seven calls now pass a literal %s format with the id as an argument, so the alert stops reappearing on every line shift.

The stripHtmlTags.ts and available-models SSRF findings were assessed and dismissed in the issue and are untouched here.

Related Issue

Closes #605

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • Performance improvement

How Has This Been Tested?

  • Unit tests
  • Integration tests
  • E2E tests
  • Manual testing

New tests: lib/utils/url.test.ts (including a 200k-slash input that would time out under the regex), a Word-preview test that renders javascript:, data: and vbscript: links and checks they lose their href while an https: link gets target and rel, and an adapter test that a base URL with trailing slashes still requests .../v1/messages. testRunService.test.ts assertions were updated for the new console.error argument shape.

The full pnpm precommit chain (ESLint, type-check, Prettier, case-status check, full Vitest suite, docs build) passes locally.

Test Configuration:

  • OS: macOS
  • Browser (if applicable): n/a
  • Node version: 24.17.0

Checklist

  • My code follows the project's style guidelines
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published
  • I have signed the CLA

Screenshots (if applicable)

n/a

Additional Notes

  • Every alert named in the issue was already dismissed as "false positive" on 2026-09-08, so nothing reopens on GitHub; this PR is the record of the verdicts.
  • Merging under the ci: title cuts no release.
  • The push trigger costs two roughly 40-minute ubuntu-latest jobs per qualifying push to main or release/**.
  • Not done here: bookmark links (#anchor) in Word previews resolve to http(s): and still open in a new tab, which predates this change; and the build-cli job in cli-release.yml holds contents: write it does not use.

…g findings

- a11y-smoke.yml now runs on push to main and release/**, so a violation
  can no longer land unscanned between pull requests
- every workflow declares a top-level permissions block (contents: read)
- OfficeDocumentPreview keeps only http, https, mailto and tel links in
  rendered Word documents instead of stripping a javascript: prefix
- trailing-slash normalisation of admin-supplied LLM base URLs uses a
  loop helper rather than a regex that backtracks on long slash runs
- forecast and test-run console.error calls pass ids as format arguments

Closes #605
The in-memory fallback buckets on Math.floor(Date.now() / 1000). A2 made
50 awaited calls on the real clock, so on a slow runner the loop crossed
a second boundary, the counter restarted, and the 50th call reported 4
remaining instead of 0. C1 had the same exposure over three calls. Both
now freeze the clock with fake timers, as B2, C2 and C3 already do.
@therealbrad
therealbrad merged commit 548b808 into main Sep 9, 2026
9 checks passed
@therealbrad
therealbrad deleted the fix/post-1.0-hardening branch September 9, 2026 15:40
@therealbrad

Copy link
Copy Markdown
Contributor Author

🎉 This PR is included in version 1.0.5 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Post-1.0 hardening: CI gaps and deferred code-scanning findings

1 participant