Repository navigation
fix: resolve Sonar security gate (ReDoS + workflow supply-chain hardening) - #85
Conversation
The AI attribution patterns used `[^<]+\s+<` where `\s` is a subset of `[^<]`, creating adjacent overlapping quantifiers with super-linear backtracking (Sonar typescript:S8786). Fold the whitespace into the single `[^<]+` run before `<` so each pattern matches in linear time. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
Address Sonar GitHub Actions security findings: - S8232: drop forgeable `github.actor` checks from workflow_dispatch conditions in lockfile-refresh.yml; manual dispatch is already restricted to write-access users by GitHub. - S6505/S8543: run npx tools with `--ignore-scripts` and pinned exact versions (@cyclonedx/cyclonedx-npm@5.0.0, license-checker@25.0.1). - S8544: enforce the lockfile on `uv run` (`--frozen`) and pin the `uvx --from cyclonedx-bom==7.1.0` tool version. - S8541: add `--no-build` to `uvx` invocations (no local project to build). `uv sync`/`uv run` keep building the local editable workspace package, so `--no-build` is not applied there. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
@codex review |
There was a problem hiding this comment.
Pull request overview
This PR addresses SonarCloud security findings by tightening potentially vulnerable regex usage in the Node commitlint rule and hardening CI workflows against supply-chain/script-execution risks, while also enforcing lockfile fidelity for Python steps.
Changes:
- Updates the Node commitlint AI-attribution footer regex patterns to remove the overlapping-quantifier ReDoS smell.
- Hardens GitHub Actions workflows by pinning
npxtool versions, adding--ignore-scripts, and pinning/lockinguv/uvxbehavior (--frozen,--no-build). - Simplifies
lockfile-refresh.ymlworkflow_dispatchgating by removing the actor equality check.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/node-commitlint/src/rules/rai-footer-exists.ts | Adjusts AI attribution footer regex patterns to address ReDoS finding. |
| .github/workflows/test-and-build.yml | Runs Python tooling/tests via uv run --frozen to enforce lockfile usage. |
| .github/workflows/security-audit.yml | Pins and hardens SBOM/license tooling and uses uv sync --frozen for lockfile enforcement. |
| .github/workflows/release-please.yml | Hardens release workflow commands (uv run --frozen, pinned npx/uvx with --ignore-scripts / --no-build). |
| .github/workflows/lockfile-refresh.yml | Removes forgeable github.actor checks from workflow_dispatch conditions. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3c59f4f9a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous ReDoS fix over-loosened the patterns: `[^<]+` absorbed all separators, so footers with no name (`Generated-by: <a@b>`), no space before the email (`Generated-by: AI<a@b>`), or values spanning multiple lines passed validation, diverging from the Python rule's `^[^<]+ <[^>]+>$` contract. Require a non-empty name and whitespace before `<email>`, and exclude `\r`/`\n` from the name/email runs so a footer cannot match across lines. The pattern stays linear: no two adjacent quantifiers share a character class. Adds regression tests for the three cases above. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
`npx @cyclonedx/cyclonedx-npm@5.0.0` is not installed in the workspace, so modern npm prompts before downloading and would hang the release job. Add `--yes`, matching the license-checker invocation. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b75ca092f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The patterns allowed a missing space after the colon (e.g. `Generated-by:AI <ai@example.com>`), which contradicts the documented `Key: Value` footer format and the rule's own error-message examples. Require at least one non-newline whitespace after the colon and add a regression test for the no-space-after-colon case. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
The license audit's `uv sync --frozen` would install from a possibly stale `uv.lock`, so newly declared production dependencies could be omitted from the CycloneDX license report scanned from `.venv`. Use `--locked` to fail the audit if the lock is out of date with `pyproject.toml`; this still satisfies the lockfile-enforcement check. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. 🚀 Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Fold in the open Dependabot dependency PRs and regenerate the lockfile: - root dev deps: @commitlint/cli & config-conventional 21.0.1 -> 21.0.2, @typescript-eslint/eslint-plugin 8.59.4 -> 8.61.1, eslint 10.4 -> 10.5, lefthook 2.1.7 -> 2.1.9, prettier 3.8.3 -> 3.8.4, vitest 4.1.6 -> 4.1.9 (PR #83); @types/node 25.9 -> 26.0 in node-commitlint (PR #84) - transitive vite resolves to 8.1.0, clearing the high-severity server.fs.deny bypass and the launch-editor NTLM advisories (PR #81) - override js-yaml ^4.2.0 and markdown-it ^14.2.0 so markdownlint-cli2's pinned transitives pick up the quadratic-DoS fixes; linkify-it and brace-expansion resolved via the regenerated lock `npm audit` now reports 0 vulnerabilities; lint, tests, and build pass. Generated-by: Claude Opus 4.8 <noreply@anthropic.com>
6f41d8c to
9cdb0a5
Compare
What 🔧
Resolves the failing SonarCloud quality gate (
new_security_rating=3) and the ReDoS code smell, plus hardens GitHub Actions supply-chain commands.Code
typescript:S8786(ReDoS) —rai-footer-exists.tsattribution regexes used[^<]+\s+<, where\s ⊂ [^<]→ adjacent overlapping quantifiers with super-linear backtracking. Folded whitespace into the single[^<]+run. Linear now; all tests + 100% coverage retained.Workflows
S8232— dropped forgeablegithub.actor == 'anchildress1'checks fromworkflow_dispatchconditions inlockfile-refresh.yml. Manual dispatch is already gated to write-access users by GitHub, so this removes the finding without weakening real protection.S6505/S8543—npxtools now run with--ignore-scriptsand pinned exact versions (@cyclonedx/cyclonedx-npm@5.0.0,license-checker@25.0.1).S8544—uv runenforces the lockfile (--frozen);uvx --from cyclonedx-bom==7.1.0pinned.S8541—--no-buildadded touvxinvocations.Known gaps 🚧
S8541onuv sync/uv run—--no-buildis not applied to these. Verified locally thatuv sync --no-buildfails (gitlint-rai @ editable+. can't be installed ... marked as --no-build but has no binary distribution): the local editable workspace package must build. All third-party deps ship wheels, so no untrustedsetup.pyexecutes. These findings will be marked Accepted in Sonar with this justification.package-lock.json— self-detection, not a real vulnerability.Test 🧪
make ai-checksgreen (clean/install/format/lint/test/benchmark/build).actionlintclean (only pre-existing info-level shellcheck notes on untouched lines).uvx --no-build --from cyclonedx-bom==7.1.0✅,uv run --frozen✅,uv sync --no-build❌ (documented above).Generated-by: Claude Opus 4.8 noreply@anthropic.com