[code-scanning-fix] Fix go/incorrect-integer-conversion: integer truncation in stableHash

[github-actions]

Generated by PR Description Updater for #42696 · 40 AIC · ⌖ 6.83 AIC · ⊞ 4.7K · ◷

[github-actions]

Great catch on the stableHash integer truncation in pkg/parser/schedule_fuzzy_scatter.go — the uint32 cast was a real latent risk on 64-bit platforms, and the int64 arithmetic replacement plus the modulo <= 0 divide-by-zero guard are the right moves here.

One thing to shore up before merge:

• Add tests for the new code paths — the modulo <= 0 guard and the int64-safe arithmetic are new behaviors with no test coverage. A few targeted unit tests for stableHash (e.g., modulo = 0, a large modulo value exceeding math.MaxUint32, and a normal small value for regression) would confirm correctness and protect against future regressions.

If you'd like a hand, you can assign this prompt to your coding agent:

Add unit tests for the stableHash function in pkg/parser/schedule_fuzzy_scatter.go.
Cover the following scenarios:
1. modulo = 0 — should return 0 (guard clause).
2. modulo < 0 (e.g. -1) — should return 0 (guard clause).
3. modulo = a normal small value (e.g. 7) — should return a value in [0, 7) and be stable across calls with the same input string.
4. modulo > math.MaxUint32 (e.g. math.MaxUint32 + 1) — should not truncate; result must be in [0, modulo) and consistent with the int64 arithmetic path.
Place tests in pkg/parser/schedule_fuzzy_scatter_test.go (create if it does not exist).

Generated by ✅ Contribution Check · 217.3 AIC · ⌖ 27.5 AIC · ⊞ 6.3K · ◷

[github-actions]

🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅

[github-actions]

✅ PR Code Quality Reviewer completed the code quality review.

[github-actions]

✅ Test Quality Sentinel completed test quality analysis.

No test files were added or modified in this PR. Test Quality Sentinel skipped.

[github-actions]

✅ Design Decision Gate 🏗️ completed the design decision gate check.

No ADR enforcement needed: PR #42696 does not have the 'implementation' label and has only 6 new lines of code in business logic directories (threshold: 100).

[Copilot]

Pull request overview

Fixes a high-severity CodeQL alert in the schedule scattering logic by removing an unsafe integer conversion in stableHash, ensuring correct modulo behavior even when modulo exceeds math.MaxUint32 on 64-bit platforms.

Changes:

• Added a modulo <= 0 guard to prevent modulo-by-zero panics.
• Replaced uint32(modulo) modulo arithmetic with int64 arithmetic to avoid truncation on 64-bit systems.

Show a summary per file

File	Description
pkg/parser/schedule_fuzzy_scatter.go	Fixes integer truncation in stableHash by switching to int64 modulo arithmetic and guarding non-positive modulus values.

Review details

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Low

[github-actions]

Review: Fix go/incorrect-integer-conversion in stableHash

The fix correctly addresses the CodeQL alert (CWE-681) with two improvements:

1. Type safety: Replaces uint32(modulo) with int64 arithmetic — int64(h.Sum32()) % int64(modulo) — preserving full precision on 64-bit platforms where modulo could exceed math.MaxUint32.

2. Defensive guard: Adds modulo <= 0 check before the division to prevent a divide-by-zero panic.

Correctness: All existing callers pass small positive values (e.g., len(slice), constants like 7, 50, 120), so behavior is unchanged for current usage.

Note: int64(h.Sum32()) converts a uint32 value (always non-negative) to int64, which is always safe. The final int() cast is safe because the modulo result is in [0, modulo) and modulo fits in int.

The fix is minimal, correct, and appropriate.

🧵 Reviewed using Impeccable skills by Impeccable Skills Reviewer · 22.8 AIC · ⌖ 6.41 AIC · ⊞ 4.9K

[github-actions]

Skills-Based Review 🧠

Applied /diagnosing-bugs and /tdd — requesting changes on test coverage gaps.

📋 Key Themes & Highlights

Key Themes

• Missing regression test for the CVE path: The vulnerability-triggering path (modulo > math.MaxUint32) is not exercised in the test suite. A future refactor could silently reintroduce the bug.
• New guard branch not tested: The modulo <= 0 guard (zero/negative) is a sound defensive addition but has zero test coverage, leaving the new branch unspecified.

Positive Highlights

• ✅ Correct root-cause fix: using int64 intermediate arithmetic cleanly sidesteps the truncation on all platforms
• ✅ Inline comment clearly explains why int64 is used and when it matters (64-bit platforms)
• ✅ Defensive modulo <= 0 guard eliminates a latent panic that was present before
• ✅ No behavioral change for existing callers (all pass small positive values ≤120)

🧠 Reviewed using Matt Pocock's skills by Matt Pocock Skills Reviewer · 37.6 AIC · ⌖ 7.58 AIC · ⊞ 6.6K
_{Comment /matt to run again}

[github-actions]

🔎 Code quality review by PR Code Quality Reviewer · 60.5 AIC · ⌖ 8.57 AIC · ⊞ 1.6K
_{Comment /review to run again}

[pelikhan]

@copilot run pr-finisher skill