feat: add vulnerability class reference and deterministic entry-point inventory - #200
Open
Haiduongcable wants to merge 4 commits into
Open
feat: add vulnerability class reference and deterministic entry-point inventory#200Haiduongcable wants to merge 4 commits into
Haiduongcable wants to merge 4 commits into
Conversation
The standard repository and scoped-path scan routes away from the discovery checklist and relies on a single six-category sentence for its entire vulnerability taxonomy. Concurrency and TOCTOU, workflow injection, prototype pollution, ReDoS, mass assignment, business-logic and authorization gaps, CORS, token validation, infrastructure configuration, and GraphQL had no coverage anywhere in the skills or references. Add a shared class reference and wire it into both the standard scan procedure and discovery so diff scans reach it as well. Each entry names the code shapes that make the class visible in source and the specific control that defeats it, so an instance can only be suppressed by naming that control. Also mark reviewed source as untrusted data in the standard scan procedure, which previously read every byte of the target without saying so.
Nothing in the plugin knows where untrusted input first reaches a target. No script mentions any web framework, and the ranking worklist only validates a model-supplied score, so a standard scan reads every file with no reachability prior at all. Add a model-free inventory that reports HTTP routes, GraphQL servers, serverless handlers, message consumers, server binds, and CI workflow triggers across Python, JavaScript, TypeScript, Go, Java, Kotlin, Ruby, PHP, and C#, with the handler symbol resolved where the syntax carries one. Wire it into the standard scan procedure to order review by reachability. Output is sorted and deduplicated so repeated runs are byte-identical, and every read is bounded. The inventory deliberately does not reuse two shared worklist constants, because both hide remote surface: generate_rank_input.EXCLUDED_DIRS contains .github, so workflow triggers are invisible to worklist-driven scans, and rank_preview.TEXT_CODE_EXTENSIONS omits .tf. State it as a prior, not a verdict: the procedure keeps reviewing every file in scope, since a file with no entry point can still hold a reachable bug through a helper.
|
Note You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
Author
|
Full matrix results from the fork PR at
Zero failures across all eight legs. Local gate in CI order, on the same commit:
The branch merges cleanly into |
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.
Summary
Two related detection-surface additions to the bundled plugin. They are separable and I am happy to
split them into one PR per concern on request.
Impact
The standard repository and scoped-path scan routes away from the discovery checklist
(
skills/finding-discovery/SKILL.md) and relies on a single sentence for its entire vulnerabilitytaxonomy (
repository-wide-scan.md): "unsafe command execution, unsafe parsing, XSS,attacker-controlled network requests, unsafe file access, and missing permission checks."
Ten classes had zero mentions anywhere in
skills/orreferences/, verified by grep: raceconditions and TOCTOU, CI/CD workflow injection, prototype pollution, ReDoS, mass assignment,
business-logic and authorization gaps, CORS, token validation, infrastructure and container
configuration, and GraphQL.
Separately, nothing in the plugin knew where untrusted input first reaches a target. No script
mentions any web framework, and
generate_rank_input.pyonly validates a model-suppliedscore,so a standard scan reads every file with no reachability signal at all.
http_route,graphql,serverless_handler,message_consumer,server_bind,ci_triggeracross 9 languagesChanges
references/vulnerability-classes.md(new). Per class: the code shapes that make it visible insource, and the specific control that defeats it. Suppression requires naming that control — "the
framework probably handles it" is explicitly not one. Wired into
repository-wide-scan.mdand intofinding-discovery/SKILL.mdbefore the workflow split, so diff scans reach it too.scripts/inventory_entry_points.py(new, model-free). Emits sorted, deduplicated JSONL plus asummary. Resolves the handler symbol where the syntax carries one. Every read is bounded
(
MAX_FILES,MAX_FILE_BYTES,MAX_TOTAL_BYTES,MAX_ROWS, line truncation). Wired into thestandard scan procedure to order review by reachability, stated explicitly as a prior and not a
scope filter — every file in
in_scope_files.txtis still reviewed.tests-ts/entry-point-inventory.test.ts(new). 12 cases shelling the real script viaspawnSync(python, ["-I", "-B", ...]), following the existingscan-recovery.test.tspattern.plugin-files.json. Registers both new files;check-package.mjsrejects unregistered ones.Two shared constants deliberately not reused
Both hide real remote surface, and the tests assert the deviation as a regression guard:
generate_rank_input.EXCLUDED_DIRScontains.github, so workflow files are invisible toworklist-driven scans. Verified:
path_is_excluded(Path(".github/workflows/x.yml"))isTrue. Aworkflow trigger is among the most directly attacker-reachable entry points a repository has.
rank_preview.TEXT_CODE_EXTENSIONSomits.tf, so Terraform is invisible.Verification
Local, in CI order:
run types— cleanrun test— 722 pass / 5 skip / 0 fail (33 files)run format— cleanrun build— cleanpnpm pack+run check:package— validates 97 bundled plugin files, 181 tarball entriesA/B comparison of the class reference
Measured directly rather than asserted. Two whole-repo scans of the same fixture commit, same model
(
gpt-5.6-sol), same effort (xhigh), same concurrency, with the plugin tree differing in exactlythe three changed files and nothing else. The fixture is a 9-file, 385-line service carrying 19
planted instances across the ten newly covered classes, with negative controls alongside them.
The reference closed both remaining gaps and raised classes the baseline never surfaced:
CWE-200) — enabled unconditionally in the fixture and unreported bythe baseline.
ENVlayer, each filed as its own candidate rather than folded into the compose finding.CWE-732).It also decomposed authorization far more precisely: six per-resolver candidates for GraphQL
field-level authorization where the baseline filed one, each naming the specific resolver
(
User.apiKeys,User.invoices,Organization.members,Organization.billingContact,User.organization, and the top-levelorganizationquery) that resolves without a tenant check.Both arms independently flagged an
Origin-header authorization gap that the fixture author hadmislabelled as safe — the scanner was right and the answer key was wrong.
Scope: both runs were terminated by content-safety filtering (#56) after discovery and before
validation, so these are discovery candidates rather than validated findings, and the richer
treatment output carried a 66% higher estimated spend. One run per arm.
Notes
CI. All eight matrix legs pass on a fork PR at
6fa68d1, so the full matrix is already greenwithout needing a workflow approval here:
ubuntu-latestx node 22 / 24 / 24.0.0 / 26 / 26.0.0,macos-latestx node 22,windows-latestx node 22 (6m28s), and thelinux-amd64container job.Zero failures. Exact run links are in the follow-up comment.
Known overlap with in-flight work. #88 and #71 both modify
skills/security-scan/references/repository-wide-scan.md, which this branch also touches to wire inthe two new artifacts. The wiring is a 10-line addition and a one-line pointer; I am glad to rebase
onto whichever of those lands first, or to drop the wiring entirely and ship the script and reference
unwired if you would rather keep that file untouched while it is being reworked.
Projection.
references/andskills/look like a projection of an internal source rather thanthe editable original. Both wiring edits are deliberately minimal for that reason, and they would
need mirroring internally to survive a sync. If the reference file itself is better raised as an
issue than carried as a PR, say so and I will move it.
Scope of the A/B evidence. Both scans were cut short by content-safety filtering (#56) after
discovery and before validation, so the comparison above reports discovery candidates rather than
validated findings, and it is one run per arm on a fixture I wrote. The entry-point inventory's
correctness, determinism, and bounds are verified independently of any model spend.
Not changed deliberately. No schema changes, no TypeScript source changes,
_sarif_resultandSARIF_LEVELSuntouched, andgenerate_rank_input.py/rank_preview.pyleft alone even though theinventory documents two of their constants as blind spots -- changing shared worklist constants is a
separate decision that belongs with you, not bundled into a new-file addition.