Skip to content

CI-SEC-CODEQL: close CodeQL alerts #57 and #59 - #1581

Merged
mergify[bot] merged 2 commits into
mainfrom
codex/ci-sec-codeql-57-59
Oct 15, 2026
Merged

mergify[bot] merged 2 commits into
mainfrom
codex/ci-sec-codeql-57-59

Conversation

@Cuarroc

@Cuarroc Cuarroc commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Diese Änderung schließt die beiden offenen CodeQL-Meldungen #57 und #59 durch Codekorrekturen, nicht durch Verwerfen. Der SVG-Export entfernt Kommentare jetzt so lange, bis kein Kommentar-Anfang mehr übrig ist, und der red-first-Beweisschritt in der CI nennt das Pull-Request-Ereignis ausdrücklich in seiner Bedingung. Die Prüfung durch zwei fremde Modellfamilien steht noch aus (Stufe A).

Report

Plan-ID: CI-SEC-CODEQL.

Was geändert wurde

  • Commit 04d8dca2 → Alert merge queue: checking #51 + #50 + #35 together on main (8ca95da) #59 (actions/cache-poisoning/poisonable-step, ci.yml, Job linux_1): Der Schritt red-first - proof against merge base (checkt pull_request.head.sha aus) hat in seinem if: jetzt github.event_name == 'pull_request' &&. Der Schritt red-first - plan hatte das schon; es gibt in ci.yml keinen weiteren Schritt mit pull_request.head.sha. scripts/ci/ci-shape.sh prüft die Bedingung nun für beide Schritte, scripts/test-ci-shape.sh hat dazu einen Mutationsfall.
  • Commit 6260cea5 → Alert chore(deps): bump the npm group across 1 directory with 5 updates #57 (js/incomplete-multi-character-sanitization, docs/diagrams/export-svg.mjs:124): Neues docs/diagrams/strip-comments.mjs mit stripComments(): wiederholt das Entfernen bis der String stabil ist und entfernt auch ein unterminiertes <!-- (Regex <!--(?:[\s\S]*?-->)?). export-svg.mjs nutzt es. Eigene Datei, damit der Test ohne Chromium und ohne argv läuft.

Belege

  • Red-first: Test-First: scripts/test-ci-shape.sh::ci-shape fails when the proof step 'if' drops the pull_request event check und Test-First: docs/diagrams/export-svg.test.mjs::stripComments leaves no comment opener behind; bash scripts/ci/red-first.sh → Exit 0 (beide an der Merge-Base rot/fehlend, am Kopf grün).
  • node --test docs/diagrams/export-svg.test.mjs → Exit 0 (6 Tests, mit Chromium).
  • bash scripts/ci/ci-shape.sh → Exit 0; bash scripts/test-ci-shape.sh → Exit 0.
  • bash scripts/ci/gates.sh lane prepush → Exit 0 (ungepiped, 279 s).

NICHT ABGEDECKT

  • Ob beide CodeQL-Alerts tatsächlich schließen, zeigt erst der CodeQL-Lauf auf main nach dem Merge; ob CodeQL die Schleife in stripComments und die explizite Ereignisbedingung als Behebung wertet, ist nicht lokal belegt.
  • Das Verhalten des Beweisschritts in einem echten Pull-Request-Lauf (Bedingung wird in GitHub Actions ausgewertet) ist lokal nicht beobachtbar.
  • Die #[cfg(windows)]-Tests und die Windows-Arme von clippy laufen unter Linux nicht; die Windows-Bahn läuft in der Merge-Queue.
  • Der Rest von export-svg.mjs (Browserpfad) ist unverändert; der Kommentar-Fall <!<!---->--> lässt sich über den Browser-Export nicht erzeugen (XMLSerializer maskiert <), daher testet der Test stripComments direkt.

Architecture drift: keine Befunde.

Review: ausstehend (Stufe A: Sicherheit + .github-Workflow, zwei Modellfamilien außer Claude). ci.yml gehört laut CODEOWNERS dem Nutzer (Review-Anfrage, blockiert nicht).

🤖 Generated with Claude Code

Review disposition (Root, head 6260cea)

Tier A (security + workflow). Author family: Anthropic (Claude Sonnet 5.5; the job was rerouted by the Codex brake). Two reviewers from other vendors, both APPROVE on 6260cea:

  • OpenAI Codex gpt-6-astra (review-pr1581-codex.md): no findings. It confirmed red-first against the base for both alerts: the old regex fails on <!<!---->-->, and the old ci-shape check accepts the guard mutation.
  • Moonshot Kimi (OpenRouter second-review lane, pr1581-kimi-6260cea5.md): five low findings, listed below.
ID Source Severity Finding Disposition
K1 Kimi low strip-comments.mjs:6 also strips comment-like text in content Follow-up. The input is our own generated SVG, and the behaviour is unchanged from the old regex.
K2 Kimi low ci.yml:352: the proof step is now skipped on push Rejected. The step already required rf_plan success, and rf_plan only runs on pull_request, so push behaviour is unchanged. No required check depends on this step outside PRs.
K3 Kimi low export-svg.test.mjs:107: no exact output assertion Follow-up.
K4 Kimi low test-ci-shape.sh:188: the mutation test covers only the proof step Follow-up.
K5 Kimi low ci-shape.sh:178: assumes 8-space indent Follow-up.

The alerts close only after the next CodeQL scan of main. Both were still open at review time, which the Codex review confirms.

…eQL #59)

The proof step checks out github.event.pull_request.head.sha; CodeQL
(actions/cache-poisoning/poisonable-step) could not see statically that it
never runs on schedule/workflow_dispatch. Add the explicit event check and
pin it in ci-shape for both red-first steps.

Test-First: scripts/test-ci-shape.sh::ci-shape fails when the proof step 'if' drops the pull_request event check
A single replace pass can re-assemble a comment opener, e.g. `<!<!---->-->`.
Move the strip into stripComments(), which repeats until stable and also
drops an unterminated opener.

Test-First: docs/diagrams/export-svg.test.mjs::stripComments leaves no comment opener behind
@Cuarroc
Cuarroc marked this pull request as ready for review October 15, 2026 17:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-15T18:02:39.838928Z 6260cea Draft marked ready
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@Cuarroc Cuarroc added the review-ok Pipeline: review disposition complete, no open high finding label Oct 15, 2026
@Cuarroc

Cuarroc commented Oct 15, 2026

Copy link
Copy Markdown
Owner Author

Root: review-ok bound to head 6260cea. Tier A: Codex (OpenAI) APPROVE and Kimi (Moonshot) APPROVE; the author is Claude (Anthropic). All checks green incl. CodeQL and GitGuardian at 2026-10-15T18:11:19Z. No open high finding.

@Cuarroc

Cuarroc commented Oct 15, 2026

Copy link
Copy Markdown
Owner Author

Root auto-queue: head 6260cea unchanged since review-ok, all checks green. @Mergifyio queue

@mergify mergify Bot added the queued label Oct 15, 2026
@mergify mergify Bot mentioned this pull request Oct 15, 2026
30 of 46 tasks
@mergify
mergify Bot merged commit ded1eef into main Oct 15, 2026
25 checks passed
@mergify

mergify Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • ✅ Entered queue — 2026-10-15 18:11 UTC · Rule: default · triggered by merge protections
  • ✅ Checks passed · on draft merge queue: checking #1581 on main (3ad56a8) #1583
  • ✅ Merged — 2026-10-15 18:25 UTC · at ded1eef530ea8d618596805c99329454d827d9d9 · merge

This pull request spent 13 minutes 46 seconds in the queue, including 13 minutes 3 seconds running CI.

Required conditions to merge
  • check-success = gates (linux)
  • check-success = gates (windows)
  • check-success = red-first
  • github-review-approved [🛡 GitHub repository ruleset rule main-pr-required-checks]
  • label != do-not-merge
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/gates (linux)
    • check-neutral = @github-actions/gates (linux)
    • check-skipped = @github-actions/gates (linux)
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/gates (windows)
    • check-neutral = @github-actions/gates (windows)
    • check-skipped = @github-actions/gates (windows)
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-success = @github-actions/red-first
    • check-neutral = @github-actions/red-first
    • check-skipped = @github-actions/red-first
  • any of [🛡 GitHub repository ruleset rule main-pr-required-checks]:
    • check-neutral = @mergify/Mergify Merge Protections
    • check-skipped = @mergify/Mergify Merge Protections
    • check-success = @mergify/Mergify Merge Protections

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

Labels

review-ok Pipeline: review disposition complete, no open high finding

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant