fix(parsers): apply ADR 0004 security hardening across 10 parsers#695
Merged
fix(parsers): apply ADR 0004 security hardening across 10 parsers#695
Conversation
Audit all ~90 parsers against ADR 0004 and remediate divergences: - Add RecursionGuard with depth tracking and cycle detection to bun_lockb.rs - Add depth parameters to clojure.rs form_to_json/format_license - Add MAX_RECURSION_DEPTH caps to gradle.rs bracket/paren/brace matching - Add .take(MAX_ITERATION_COUNT) to python.rs and conda.rs iteration loops - Replace .unwrap() with .expect() in cpan_makefile_pl.rs and rpm_specfile.rs - Replace panic! with Result return in rpm_db.rs native_kind_for_datasource - Replace .unwrap() with safe alternative in nix.rs Vec::pop() - Add path-traversal check to nuget.rs read_nupkg_license_file - Add total uncompressed size accumulation to nuget.rs extract_nupkg_archive
98f30c1 to
0e1c187
Compare
0e1c187 to
7ef680e
Compare
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.
Summary
Audited all ~90 parser source files against ADR 0004 (Security-First Parsing), remediated every divergence found, and extracted a shared
RecursionGuard<K>utility now adopted across 17 parsers. Two parser families (Ruby/Cocoa/Haskell and Python/Conda/Pixi after fixes) were already fully compliant.Changes by Parser
bun_lockb.rs— Recursion depth tracking + cycle detection (Critical)RecursionGuardstruct encapsulatingdepthcounter andHashSet<usize>visited setbuild_dependencies_for_packageandbuild_resolved_packagenow accept&mut RecursionGuard<usize>instead of rawdepth/visitedparamswarn!()when depth exceedsMAX_RECURSION_DEPTH(50)guard.enter(pkg_idx)checks if already visited; if so, skips withwarn!()and returnsNoneguard.leave(pkg_idx)removes the index from visited, allowing diamond dependencies in different branchesclojure.rs— Unbounded recursion in AST conversion (High)form_to_json()andformat_license(): added depth tracking; return early withwarn!()when depth exceededRecursionGuard<()>viadescend()/ascend()in struct and free functionsgradle.rs— Unbounded nesting depth in delimiter matching (Low)warn!()and breaking with partial resultpython.rs/conda.rs— Unbounded iteration.take(MAX_ITERATION_COUNT)to iteration loops inparse_installed_files_txt(),parse_sources_txt(), andextract_pip_dependencies()rpm_db.rs—panic!()in library codeResult<..., String>; replacedpanic!()withErr(...)rpm_specfile.rs/cpan_makefile_pl.rs—.unwrap()on compile-time regexes.unwrap()with.expect("valid regex: ...")nix.rs—.unwrap()onVec::pop()unwrap_or_else(|| Expr::Symbol(String::new()))nuget.rs— Archive safety gapsread_nupkg_license_fileextract_nupkg_archiveRecursionGuard Extraction (follow-up commit)
Extracted the
RecursionGuardfrombun_lockb.rsintosrc/parsers/utils.rsas a generic shared utility and adopted it across 17 parsers:RecursionGuard<K: Hash + Eq>— depth + cycle detection (keyed byusize,String, orPathBuf)guard.enter(key)/guard.leave(key)— visited-set tracking + depth increment/decrementguard.exceeded()— checks depth againstMAX_RECURSION_DEPTH(50)RecursionGuard<()>— depth-only tracking (no cycle detection)guard.descend()/guard.ascend()— lightweight increment/decrement, returnstrueif exceededParsers refactored to
RecursionGuard<K>(depth + cycle detection):bun_lockb.rs(usize),nuget.rs(PathBuf),sbt.rs(String),swift_show_dependencies.rs(String+()),requirements_txt.rs(PathBuf)Parsers refactored to
RecursionGuard<()>(depth-only):dart.rs,bazel.rs,julia.rs,cargo.rs,uv_lock.rs,pylock_toml.rs,license_normalization.rs,npm_lock.rs,clojure.rs(struct + free functions),hex_lock.rs,meson.rs,nix.rsRemoved 17 duplicate
MAX_RECURSION_DEPTHconstants; all parsers now use the sharedMAX_RECURSION_DEPTHfromutils.rs.Skipped:
maven.rs,conan.rs,python.rs— these have domain-specific extras (caches, node counts, resolution stacks) beyond simple depth+visited tracking.Skill file update
Updated
.opencode/skills/add-parser/SKILL.mdto documentRecursionGuardin the security utilities section so future parser implementations use it from the start.Testing
cargo checkpassescargo clippypasses with zero warningsADR Compliance
After this PR, all ~90 parser files comply with ADR 0004 across all 6 requirements:
.unwrap()in library code