fix(ci): restore semgrep to ci-fast, add withdrawal binding tests#337
Merged
Conversation
- Add missing inSecret0Upper input to MARKPool.fast.mjs happy-path test - Increase ci-fast timeout to 1200s for cold Solidity compilation - Move Semgrep to standalone task scoped to contracts/ and circuits/ - Remove orphaned circuits/scripts/test-fast.mjs
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
- Restore Semgrep scan with --severity ERROR filter to ci-fast task - Change circuits step from pnpm test to pnpm test:fast for faster iteration - Add RELAYER_MAX constant and withdrawal binding test cases in MARKPool.fast.mjs - Add happy path test for withdrawal (exercises inSecret0Upper constraint) - Add sad path test for tampered withdrawOwner (attack POC)
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
- ci-fast: add --error to semgrep so ERROR findings cause non-zero exit - semgrep task: restore --config=auto --config=p/security-audit and --error (lost when the task was previously refactored) - MARKPool.fast.mjs: add forged-upper owner-binding bypass test that computes inSecret0Upper = (secret - spoofedOwner) * RELAYER_MAX^-1 mod p and verifies the Num2Bits(93) range check rejects the out-of-range value Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
Previous withdrawal test used secret=111n which is smaller than 2^160, so inSecret0Upper was always 0n and the upper-bits term of the binding constraint was never exercised. Add a second happy-path case using secret = RELAYER_MAX + 42n (upper=1n, lower=42n) so a regression that ignores inSecret0Upper or uses the wrong multiplier would be caught. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
The fast gate previously ran --config=auto alongside the MARK custom rules. This was dropped when semgrep was re-added to ci-fast, narrowing the scan to only project-specific rules and letting generic security findings through. Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
…o fast suite - ci-fast/semgrep tasks: remove contracts/ circuits/ target restriction so frontend JS/TS is scanned again (matches historical no-target invocation; .semgrepignore handles build artifact exclusions) - MARKPool.fast.mjs: port four security-relevant tests from the full suite that were missing from the fast gate: - withdraw amount non-zero but owner zero - withdraw amount non-zero but recipient zero - withdraw amount zero but owner non-zero - wrong output commitment Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes
Tests
All 11 fast circuit tests passing:
Security
The withdrawal binding path was previously untested in fast suite. Added coverage to prevent regression on the inSecret0Upper constraint that binds withdrawOwner to inSecret[0].
Need help on this PR? Tag
/codesmithwith what you need. Autofix is enabled.Greptile Summary
ERRORfilter.pnpm test:fast.Confidence Score: 5/5
The changes are limited to CI task wiring and additional fast circuit coverage, with no reported code issues requiring changes.
The modified paths align with the PR description and the added tests exercise the withdrawal binding behavior without introducing review findings.
What T-Rex did
Reviews (4): Last reviewed commit: "fix: restore full-repo semgrep scope and..." | Re-trigger Greptile