Repository navigation
fix(rules): Refactor rules to use per-source and per-sink tags for joins - #312
Merged
Merged
Conversation
…ta-source tag Tag the servlet, spring (general) and seam untrusted-data-source lib rules with `untrusted-data-source`, and migrate every security join that wired those sources by file path to a single `tag: untrusted-data-source` ref (collapsing the per-framework `on:` conditions into one). The spring path-specific source stays an explicit `rule:` ref (e.g. path-traversal keeps it alongside the tag). Verified: the full java ruleset still loads the same 86 rules with no tag/join resolution errors.
Give each top-level/lib sink rule a descriptive tag (ssrf-sink, command-injection-sink, sqli-sink, xxe-sink, servlet-xss-sink, ...) and switch every security join to reference its sink via `tag:`, so joins are fully declarative and other users can extend a join by tagging their own rule with the same sink tag. Each sink tag maps 1:1 to its built-in sink (no broadening). The two file-disclosure sinks were defined inline under `join.rules:`, which a `tag:` ref cannot reach (a join-local inline rule never enters the global tag index). They are promoted to top-level `lib` rules carrying their tags, so they are wired by tag like every other sink and remain extensible. Verified: the full java ruleset still loads the same 86 rules with no tag/join resolution errors.
The untrusted-data-source tag union broadened the source set of every join whose explicit refs were a subset of the tag's members, changing analysis results: path-traversal picked up the general spring source alongside the deliberately narrow untrusted-path-source and started flagging the safe non-wildcard @PathVariable sample (rule tests went from FP=0 to FP=1, findings 201 -> 203 on the test corpus). Restore behavior parity with the pre-retag ruleset: - Drop the tag from seam-untrusted-data-source, so the tag's membership is exactly the {servlet, spring} pair that every remaining tag-using join historically wired. - Revert the framework-scoped joins (servlet-only, spring-only and the seam-only seam-log-injection) to explicit rule refs; these rules encode the framework in their meaning, so a shared tag union is the wrong reference for them. Aliases and on-conditions are unchanged; each revert is a one-line ref swap. Verified: rule tests back to 332 success with FP=FN=skipped=0, and per-rule SARIF finding counts identical to the pre-retag baseline (201).
Replace the shared untrusted-data-source union tag with a distinct tag per source, so joins reference sources the same way they reference sinks (one declarative per-source tag), and drop the last file-path source refs. The union tag (servlet+spring) silently fanned every tagged source into each join using it, which is what broadened path-traversal and forced the earlier revert to explicit rule refs. With per-source tags there is no union to broaden: each join names exactly the source tags it wired before. - Tag each source lib rule: servlet-untrusted-data-source, spring-untrusted-data-source, seam-untrusted-data-source, and rename untrusted-path-source -> spring-untrusted-path-source. - Split the 20 union joins back into two explicit per-source tag refs (servlet + spring) with per-alias on-conditions -- the pre-collapse structure, now tag-based instead of path-based. - Swap the 14 framework-scoped joins from file-path rule: refs to their single per-source tag:. No source is referenced by path or a union tag anywhere; different sink joins reference different source-tag sets. No engine/loader change: a per-source 1:1 tag already resolves like a rule ref (as untrusted-path-source and every sink tag do). Verified: rule tests 332 success with FP=FN=skipped=0, identical to the pre-retag baseline.
Give each Go lib sink rule a descriptive tag (command-injection-sink, sqli-sink, path-traversal-sink, ssrf-sink, ssti-sink, xss-sink) and switch every Go security join to reference its sink via `tag:`, matching what the Java ruleset already does. Each sink tag maps 1:1 to its built-in sink, so nothing broadens; other users can extend a join by tagging their own rule with the same sink tag. Four of the six tags are spelled exactly as their Java counterparts. Tag expansion is language-scoped (a tag only expands to rules of the join's own language), so the shared spelling never mixes Java and Go rules. Verified: rule tests 536 success with FP=FN=skipped=0, byte-identical success list to the pre-retag baseline (Java + Go, go-ssa-server enabled).
Tag each Go lib source rule with a distinct tag of its own (http-untrusted-data-source, request-uri-untrusted-data-source, beego-header-untrusted-data-source) and swap the six Go security joins from file-path `rule:` refs to those tags, so sources are wired exactly like sinks and no Go join references a rule by path anymore. Per-source tags, not one shared union tag: each tag names a single source rule, so a join keeps referencing precisely the sources it wired before and different joins keep different source sets (cmdinj and sql-injection pull the extra beego-header/request-uri sources; the rest do not). Verified: rule tests 536 success with FP=FN=skipped=0, byte-identical success list to the pre-retag baseline (Java + Go, go-ssa-server enabled).
The retag left the README's join example describing a ref style the built-in rulesets no longer use: it showed `ruleset/java/security/ssrf.yaml` wiring its source and sink by file path, which that file stopped doing. Refresh the example from the current ssrf join, document `tag:` alongside `rule:` in the refs semantics, and add a Rule Tags section covering what a tag expands to (every enabled same-language rule carrying it), why that makes tags the extension point for user rules, and why the built-in per-source/per-sink tags each name exactly one rule.
misonijnik
commented
Jul 28, 2026
Co-authored-by: Aleksandr Misonizhnik <misonijnik@gmail.com>
Co-authored-by: Aleksandr Misonizhnik <misonijnik@gmail.com>
Co-authored-by: Aleksandr Misonizhnik <misonijnik@gmail.com>
Co-authored-by: Aleksandr Misonizhnik <misonijnik@gmail.com>
Co-authored-by: Aleksandr Misonizhnik <misonijnik@gmail.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
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.
No description provided.