Skip to content

feat: allow multiple rule ids and/or filenames - #246

Merged
fzipi merged 7 commits into
mainfrom
chore/add-multiple-ruleids-filenames-regex-update
Apr 26, 2026
Merged

feat: allow multiple rule ids and/or filenames#246
fzipi merged 7 commits into
mainfrom
chore/add-multiple-ruleids-filenames-regex-update

Conversation

@fzipi

@fzipi fzipi commented Jul 24, 2025

Copy link
Copy Markdown
Member

what

  • add support for passing multiple rule ids and/or filenames to the regex update command

why

  • to be able to use pre-commit passing filenames of modified files to be run

references

fzipi added 4 commits July 24, 2025 09:52
@fzipi
feat: allow multiple rule ids and/or filenames
4268596 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
fix: add helper funcs
7f20ae7 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
fix: remove unused func
0b85e52 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
@fzipi
fix: update test error names
a2b1fb2 
Signed-off-by: Felipe Zipitria <felipe.zipitria@owasp.org>
theseion
theseion requested changes Apr 25, 2026
View reviewed changes

@theseion theseion left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does pre-commit store files in a separate directory (seems weird to me)? If so, ignore my comments about not needing the file path, of course.

Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
Comment thread cmd/regex_update.go Outdated
fzipi and others added 2 commits April 25, 2026 06:56
@fzipi @theseion
Apply suggestions from code review
7b5482c 
Co-authored-by: Max Leske <250711+theseion@users.noreply.github.com>
@fzipi @claude
fix: simplify path handling and deduplicate chain offset parsing
85179ad 
Address review feedback from theseion:
- Remove isPath helper (unnecessary complexity)
- Simplify extractBasename to always use filepath.Clean + filepath.Base
- Remove filePath from parsedRuleValues struct; compute from assembly
  dir at call sites instead of supporting arbitrary directories
- Remove relative path check block from parseAndValidateArgument
- Extract parseChainOffset helper to deduplicate offset parsing logic
  used in parseRuleIdValidation, parseRuleIdToStruct, and performUpdateAll
- Update Long description to drop mention of relative path support
- Update test error assertion to match new error message

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@fzipi

fzipi commented Apr 25, 2026

Copy link
Copy Markdown
Member Author

@copilot resolve the merge conflicts in this pull request

@fzipi
merge: resolve conflicts with origin/main refactoring
ce4e5a9 
Integrate multi-rule support (multiple RULE_IDs/filenames) with the
new package structure introduced in origin/main. Key changes:

- Adapt cmd/regex/update/update.go to use New(cmdContext) factory
  while keeping multi-rule support (parsedRuleValues, extractBasename,
  parseAndValidateArgument, parseChainOffset, parseRuleIdToStruct,
  performUpdateAll, performUpdateMultiple)
- Use regexInternal.RunAssemble from the new internal package
- Return errors from processRule/updateRegex instead of logger.Fatal
- Adapt update_test.go to new package structure (s.cmd.SetArgs instead
  of rootCmd.SetArgs, s.rootDir instead of s.tempDir)
- Take main's version of cmd/regex/compare/compare.go unchanged

Co-authored-by: fzipi <3012076+fzipi@users.noreply.github.com>

Copilot AI commented Apr 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@copilot resolve the merge conflicts in this pull request

Resolved in ce4e5a9. The branch had conflicts with origin/main which refactored the code into new sub-packages (cmd/regex/update/, cmd/regex/compare/, etc.). I've merged the multi-rule support into the new package structure, adapting the tests to use the new package-level test approach (s.cmd.SetArgs instead of rootCmd.SetArgs).

Does pre-commit store files in a separate directory (seems weird to me)? If so, ignore my comments about not needing the file path, of course.

No, pre-commit doesn't store files in a separate directory. It passes the full paths of modified files as arguments. The extractBasename function handles this by stripping the path to get just the filename (e.g., /path/to/regex-assembly/932100.ra → 932100.ra), which is then looked up in the assembly directory. So the path handling works regardless of where pre-commit passes the files from.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • https://api.github.com/repos/coreruleset/crs-toolchain/releases
    • Triggering command: /tmp/go-build4086006332/b510/self_update.test /tmp/go-build4086006332/b510/self_update.test -test.testlogfile=/tmp/go-build4086006332/b510/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build4086006332/b379/vet.cfg 3458389/b090/_pkg_.a m ux-amd64/pkg/tool/linux_amd64/vet _amd64.s otgit mpile ux-amd64/pkg/too-buildtags om/c�� o 0.1-go1.25.0.lin-ifaceassert ux-amd64/pkg/tool/linux_amd64/vet cgo=false false -g&#34; &#34;-lpthread&#34; ux-amd64/pkg/too-buildtags (http block)
    • Triggering command: REDACTED, pid is -1 (http block)
  • wordnetcode.princeton.edu
    • Triggering command: /tmp/go-build4086006332/b504/fp_finder.test /tmp/go-build4086006332/b504/fp_finder.test -test.testlogfile=/tmp/go-build4086006332/b504/testlog.txt -test.paniconexit0 -test.timeout=10m0s /tmp/go-build4086006332/b368/vet.cfg .org/toolchain@v0.0.1-go1.25.0.l--gdwarf-5 UQ4ZHJ1Rk ux-amd64/pkg/tool/linux_amd64/vet (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@fzipi
fzipi marked this pull request as ready for review April 25, 2026 13:58
@fzipi
fzipi requested a review from theseion April 25, 2026 13:59
@fzipi
fzipi enabled auto-merge (squash) April 25, 2026 13:59
@fzipi
fzipi merged commit 7d548dd into main Apr 26, 2026
4 checks passed
@fzipi
fzipi deleted the chore/add-multiple-ruleids-filenames-regex-update branch April 26, 2026 06:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@theseion theseion theseion approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feature: add passing filename instead of RULE_ID for update

3 participants

@fzipi @theseion