🤖 Kimi Code Review

Review of .github/scripts/set-pr-status.js

The change addresses a potential edge case where nested HTML comments might not be fully removed from PR descriptions. However, there's a more efficient and safer approach.

Issues:

1. Performance concern (lines 154-159): The do-while loop could potentially become an infinite loop if the regex replacement doesn't converge. While unlikely with the current regex, it's a theoretical risk.

2. Inefficient approach: The iterative replacement is unnecessary. A single regex replacement with a more comprehensive pattern can handle nested comments in one pass.

Suggested improvement:

Replace the do-while loop with a single, more robust regex that handles nested comments:

// Instead of the do-while loop
const withoutComments = body.replace(/<!--(?:[^-]|-(?!->))*-->/g, "");

This regex uses a negative lookahead to properly handle nested comments without the need for iteration. It's more efficient and eliminates the infinite loop risk.

The rest of the function logic appears correct for extracting linked issue numbers from PR descriptions.

Automated review by Kimi (Moonshot AI) · custom prompt

🤖 Claude Code Review

The diff is small and self-contained. I have enough context to write the review.

PR Review: fix(ci): loop HTML comment removal to prevent incomplete sanitization

Scope: .github/scripts/set-pr-status.js — CI scripting only, no Rust/EVM code changed.

Summary

The fix correctly addresses the CodeQL js/incomplete-multi-character-sanitization alert. The loop-until-stable approach is the canonical recommendation for this class of issue.

Correctness

The loop is sound. A crafted input like <!-<!-- foo -->-> would produce <!-- foo --> after the first pass, and the loop catches that in the second iteration. The do...while structure guarantees at least one replacement pass, and the termination condition (withoutComments !== previous) correctly detects a fixed point.

One minor observation: the regex uses a non-greedy [\s\S]*? match, which means on each iteration it matches the innermost comment first. This is the correct behavior for iteratively unwrapping nested constructs — no concern here.

ReDoS / performance

The regex <!--[\s\S]*?--> has no catastrophic backtracking risk. With non-greedy matching and a clear terminating delimiter -->, the engine won't exhibit exponential behavior. For a PR body (bounded, human-authored text), performance is not a concern.

Termination

The loop always terminates: each iteration either removes at least one <!--...--> match (strictly shortening the string) or leaves the string unchanged (and the loop exits). The string length is non-increasing and bounded below by 0, so the invariant holds.

Minor suggestions

• The previous variable is declared with let but only used inside the loop for comparison. This is fine stylistically, but declaring it as let previous = body before the loop (matching the initial value of withoutComments) would make the intent slightly more explicit — though the do...while makes the logic clear enough as-is.

• Consider whether a maximum iteration guard (e.g. capping at 100 iterations) is warranted for defense-in-depth against a hypothetical deeply-nested adversarial input. Given that each iteration strictly removes content, even a deeply nested 10-level structure would only need 10 passes — this is unlikely to matter in practice, but it could be a minor hardening note.

Verdict

Approve. The fix is minimal, correct, and directly addresses the reported vulnerability class. The logic is easy to follow and introduces no regressions for normal PR bodies.

Automated review by Claude (Anthropic) · custom prompt

🤖 Codex Code Review

No blocking findings in this PR.

• The change in set-pr-status.js is behaviorally safe: repeated stripping is idempotent and avoids some malformed/nested-comment edge cases where a single pass may miss hidden segments.
• I did not identify correctness, security, or performance regressions from this diff in practical CI conditions.

Optional improvement:

1. Add a small regression test around extractLinkedIssueNumbers to lock in expected behavior for comment-heavy PR bodies (especially malformed/nested comment patterns).

Automated review by OpenAI Codex · custom prompt

Greptile Summary

This PR hardens the extractLinkedIssueNumbers helper in .github/scripts/set-pr-status.js by replacing a single-pass HTML comment strip with a do-while loop that repeats until the string stabilises, addressing CodeQL alert js/incomplete-multi-character-sanitization.

• Change is correct and safe: Each iteration of the loop can only shorten the string (by removing one or more <!--…--> matches); the loop terminates as soon as no match is found, so there is no risk of an infinite loop.
• No behavioural change for normal inputs: On typical PR bodies the loop performs exactly two iterations — one removal pass followed by one no-op confirmation pass.
• PR description example is slightly misleading: The description states that <!-<!-- foo -->-> produces <!-- foo --> after one pass, but tracing the regex shows it actually produces <!--> (five characters), which is neither a valid HTML comment nor a closes #N keyword. The practical risk was therefore even lower than described, though the fix is still the correct approach recommended by CodeQL.
• No ReDoS risk: The lazy [\s\S]*? quantifier is not susceptible to catastrophic backtracking; combined with GitHub's PR body size limit, performance is not a concern.

Confidence Score: 5/5

• This PR is safe to merge — it is a minimal, well-reasoned security hardening change with no functional risk.
• The change is a four-line, self-contained loop that is provably terminating and produces identical results for all normal inputs. It correctly resolves the CodeQL alert without introducing any new complexity or side-effects.
• No files require special attention.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A([Start: prBody]) --> B[withoutComments = body]
    B --> C[previous = withoutComments]
    C --> D["withoutComments = withoutComments\n.replace(/<!--[\s\S]*?-->/g, '')"]
    D --> E{withoutComments\n=== previous?}
    E -- No, matches found --> C
    E -- Yes, stable --> F["matchAll closes/fixes/resolves #N"]
    F --> G([Return issue numbers])

_{Last reviewed commit: f737859}

The do-while loop could potentially become an infinite loop if the regex replacement doesn't converge.

The loop always terminates: each iteration either removes at least one <!--...--> match (strictly shortening the string) or leaves the string unchanged (and the loop exits). Since the string length is non-increasing and bounded below by 0, convergence is guaranteed — there's no infinite loop risk.

A single regex replacement with a more comprehensive pattern can handle nested comments in one pass.

The loop-until-stable approach is the one recommended by CodeQL's own documentation for this exact alert type. It's also easier to reason about correctness compared to a more complex single-pass regex.