Skip to content

fix(ci): close CodeQL alerts 1 through 10 - #635

Merged
steipete merged 1 commit into
mainfrom
codex/codeql-alerts-1-10
Aug 26, 2026
Merged

steipete merged 1 commit into
mainfrom
codex/codeql-alerts-1-10

Conversation

@steipete

Copy link
Copy Markdown
Collaborator

Summary

  • declare least-privilege workflow permissions for CI, Commander, and Homebrew automation
  • replace regex-only docs heading stripping with one quote-aware structural parser
  • add discriminating TOC unit/integration coverage to test:safe, macOS CI, and the Pages trigger

Tests

  • pnpm run test:safe
  • pnpm run test:docs-site
  • node scripts/build-docs-site.mjs
  • generated docs comparison: 78 files byte-identical
  • git diff --check

The rebased head preserves the reviewed patch exactly (c966806c415f70b897c5165e4a8b6091e5b34b1d).

@steipete
steipete requested a review from a team as a code owner August 26, 2026 10:55
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

@cursor

cursor Bot commented Aug 26, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Changes are limited to CI permissions, docs-site build logic, and tests; no runtime product or security-sensitive application paths are modified.

Overview
Addresses CodeQL workflow-permission findings by tightening GitHub Actions defaults: Commander and macOS CI now declare contents: read, and Homebrew tap dispatch uses empty workflow permissions with an explicit note that cross-repo access relies on HOMEBREW_TAP_TOKEN. Pages rebuilds when scripts/docs-site-toc.mjs changes.

Docs site generation no longer strips heading HTML with broad regexes in tocFromHtml; it delegates to new renderedHeadingText in docs-site-toc.mjs, which skips the anchor link and walks complete tags with quote-aware parsing so characters like > inside attributes do not corrupt TOC labels.

test:docs-site (tests/docs-site-toc.test.mjs) covers quoted-delimiter edge cases and an end-to-end fixture build; it is wired into test:safe and the macOS CI docs lint step alongside existing docs lint.

Reviewed by Cursor Bugbot for commit ed23706. Bugbot is set up for automated code reviews on this repo. Configure here.

@clawsweeper clawsweeper Bot added P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. labels Aug 26, 2026
@clawsweeper

clawsweeper Bot commented Aug 26, 2026

Copy link
Copy Markdown

Codex review: needs maintainer review before merge. Reviewed August 26, 2026, 6:59 AM ET / 10:59 UTC.

ClawSweeper review

What this changes

The PR limits token permissions in four GitHub Actions workflows, replaces regex-based docs heading stripping with quote-aware tag scanning, and adds TOC coverage to the safe test suite and macOS CI.

Merge readiness

Ready for maintainer review

Keep open: current main still uses the fragile regex-only heading stripping, while this coherent collaborator PR adds the scoped parser, regression coverage, and least-privilege workflow declarations. No actionable patch defect was found; allow the in-progress checks to finish under normal review.

Priority: P2
Reviewed head: ed23706e9d843fdeadd2ba40a0b236da4ad33c49

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) A focused, source-consistent repair with targeted regression coverage and no actionable review finding.
Proof confidence 🌊 off-meta tidepool Not applicable: The author is a COLLABORATOR, so the external-contributor real-behavior-proof gate does not apply; the PR body nonetheless reports targeted test and generated-output validation.
Patch quality 🐚 platinum hermit (4/6) No actionable review findings were identified.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: The author is a COLLABORATOR, so the external-contributor real-behavior-proof gate does not apply; the PR body nonetheless reports targeted test and generated-output validation.
Evidence reviewed 6 items Current-main gap: Current main still removes heading markup with broad regex replacements, so the quote-delimiter edge case remains unfixed there.
Renderer contract and scoped repair: The builder owns the exact anchor-first heading HTML and now delegates TOC text extraction to the isolated quote-aware parser.
Regression coverage: The added tests cover a quoted greater-than delimiter and a fixture build that verifies generated TOC labels.
Findings None None.
Security None None.

Live Verification

Command: pnpm run test:docs-site

Result: FAIL (failed) — step 1 run pnpm run test:docs-site: previous terminal command was still running after 30 seconds: "pnpm run test:docs-site"

[WARN] Unsupported platform: wanted: {"cpu":["arm64","x64"],"os":["darwin"],"libc":["any"]} (current: {"os":"linux","cpu":"x64","libc":"glibc"})
$ node --test tests/docs-site-toc.test.mjs
✔ heading text ignores quoted tag delimiters (0.949914ms)
✔ docs TOC structurally extracts renderer-owned heading text (41.512404ms)
ℹ tests 2
ℹ suites 0
ℹ pass 2
ℹ fail 0
ℹ cancelled 0
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 104.419995

Assertions:

  • FAIL expect_output: heading text ignores quoted tag delimiters

How this fits together

The docs-site builder renders Markdown headings into HTML and derives on-page navigation from that HTML. GitHub Actions runs repository CI and release-adjacent automation; this PR narrows their token capabilities while extending docs-site validation.

flowchart LR
  A[Markdown documentation] --> B[Docs-site builder]
  B --> C[Rendered heading HTML]
  C --> D[Heading text parser]
  D --> E[On-page table of contents]
  F[Workflow definitions] --> G[GitHub Actions jobs]
  G --> H[CI and release automation]
Loading

Before merge

None.

Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Implementation and coverage delta production +59/-6, tests +42 across 8 files The added parser and workflow hardening are paired with a focused fixture-based regression test rather than broad product-runtime changes.

Technical review

Best possible solution:

Merge the focused parser and workflow-permission changes once the remaining required checks confirm the exact head.

Do we have a high-confidence way to reproduce the issue?

Yes—source shows current main’s regex strips at the first greater-than character in a quoted attribute, and the added test supplies the focused heading input that exercises the corrected parser.

Is this the best way to solve the issue?

Yes—the renderer owns the constrained HTML input, so a small quote-aware parser plus an end-to-end generated-TOC test is a narrower maintainable repair than expanding the previous regexes.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 220d43af72c3.

Labels

Label changes:

  • add P2: This is a bounded CI and documentation-generation correction with limited end-user blast radius.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The author is a COLLABORATOR, so the external-contributor real-behavior-proof gate does not apply; the PR body nonetheless reports targeted test and generated-output validation.

Label justifications:

  • P2: This is a bounded CI and documentation-generation correction with limited end-user blast radius.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: The author is a COLLABORATOR, so the external-contributor real-behavior-proof gate does not apply; the PR body nonetheless reports targeted test and generated-output validation.

Evidence

What I checked:

  • Current-main gap: Current main still removes heading markup with broad regex replacements, so the quote-delimiter edge case remains unfixed there. (scripts/build-docs-site.mjs:485, 220d43af72c3)
  • Renderer contract and scoped repair: The builder owns the exact anchor-first heading HTML and now delegates TOC text extraction to the isolated quote-aware parser. (scripts/build-docs-site.mjs:389, ed23706e9d84)
  • Regression coverage: The added tests cover a quoted greater-than delimiter and a fixture build that verifies generated TOC labels. (tests/docs-site-toc.test.mjs:13, ed23706e9d84)
  • Workflow authority boundary: The Homebrew workflow explicitly supplies HOMEBREW_TAP_TOKEN as GH_TOKEN for cross-repository dispatch, so its empty default token permissions do not remove the dispatch credential. (.github/workflows/update-homebrew.yml:12, ed23706e9d84)
  • Feature-history provenance: Blame and main-branch history attribute the original docs-site TOC implementation and these workflows to the same recent area contributor, providing a concrete routing trail. (scripts/build-docs-site.mjs:481, 7ce75192f329)
  • Patch hygiene: The eight-file diff is clean under Git's whitespace check. (ed23706e9d84)

Likely related people:

  • steipete: Current-main history and blame tie the docs-site TOC implementation and the affected workflow files to Peter Steinberger, whose GitHub handle is the PR author. (role: original docs-site and workflow author; recent area contributor; confidence: high; commits: 7ce75192f329, ed23706e9d84; files: scripts/build-docs-site.mjs, .github/workflows/macos-ci.yml, .github/workflows/update-homebrew.yml)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Allow the in-progress CodeQL and core checks to complete for this exact head.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@steipete
steipete merged commit 1cd6464 into main Aug 26, 2026
24 checks passed
@steipete
steipete deleted the codex/codeql-alerts-1-10 branch August 26, 2026 13:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant