Detect dependency source redirection

[rng1995]

Summary

Add deterministic HIGH SC10 findings when package-manager configuration adds or replaces a dependency source, or when a dynamic destination cannot be resolved safely from simple local assignments.

The analyzer covers npm, Yarn, pip, Poetry, Maven, and Cargo across direct config files, commands/environment variables, generated heredoc configs, and actionable shell fences. It never executes configuration or contacts a registry.

Root cause

Existing generic patterns could notice credential/configuration-adjacent text but did not model package-source changes as dependency trust-boundary changes. A non-default registry or index could therefore affect dependency resolution without a dedicated rule, operation, scope, destination status, or package-manager evidence.

BEFORE behavior

For a published regression case that generates npm and Yarn configuration through a shell script:

• no dependency-source-specific finding was emitted;
• the destination and add/replace operation were absent from evidence;
• only generic findings contributed to the result;
• result: 46 / MEDIUM / CAUTION.

AFTER behavior

The same case now emits two deterministic SC10 HIGH findings:

Ecosystem	Line	Operation	Surface	Destination status
npm	51	replace	.npmrc	resolved
Yarn	58	replace	.yarnrc	resolved

The destination is resolved through a same-file literal assignment and carried as structured, credential-safe evidence. The resulting combined assessment is 95 / CRITICAL / DO_NOT_INSTALL.

Supported surfaces

Ecosystem	Direct configuration	Commands/environment	Generated configuration
npm	.npmrc, including scoped registries	npm config set, NPM_CONFIG_REGISTRY	.npmrc heredoc
Yarn	.yarnrc, .yarnrc.yml	yarn config set	Yarn config heredoc
pip	pip.conf, pip.ini	index flags, pip config set, index environment variables	pip config heredoc
Poetry	pyproject.toml sources	source/repository commands	pyproject.toml heredoc
Maven	repositories and mirrors in XML config	Maven CLI repository override	Maven XML heredoc
Cargo	source replacement and registry indexes	registry-index environment variables	Cargo config heredoc

Commands in executable scripts and shell-language Markdown fences are actionable. Explanatory prose, comments, unrelated uses of the word “registry,” and non-shell fences do not create SC10 findings.

Deterministic decision model

• Built-in canonical public defaults do not produce SC10 when left unchanged.
• A noncanonical replacement is HIGH.
• An additional source is HIGH.
• A destination that remains dynamic after simple same-file literal resolution is HIGH with status unresolved.
• Every other resolved destination is treated uniformly; there are no organization allowlists, DNS checks, network calls, or reputation judgments.
• SC10 survives optional LLM meta-analysis. Existing explicit, user-selected baseline behavior is unchanged.

Evidence and credential safety

Each finding includes:

• ecosystem;
• add/replace operation;
• configuration surface;
• global or scoped reach;
• redacted destination;
• resolved/unresolved status.

URL userinfo and sensitive query values are removed from SC10 findings. Report-level defense in depth applies the same redaction to every finding field and evidence string in terminal, JSON, Markdown, and SARIF output.

Validation

• Dependency-source and report-redaction suite: 24 passed.
• Supply-chain, meta-analysis, and reporting regression suites: 231 passed.
• Full suite on the combined stack: 2,226 passed, 13 skipped, 38 deselected, 4 xfailed.
• Source distribution and wheel build passed; both new analyzer modules are present in the wheel.
• Focused mypy, changed-file Ruff formatting/lint, and git diff --check passed.

Review order

This PR is intentionally stacked on the nested-artifact PR so each change remains reviewable. After the first PR merges, this PR can be retargeted to main; its own commit contains only SC10, credential redaction, tests, and documentation.

Out of scope

• Registry reputation, ownership, reachability, or vulnerability lookup.
• Executing package-manager commands or configuration.
• User-managed trusted-host allowlists.

[rng1995]

Review comments taken up offline with Nir Paz and addressed in the PR.

[rng1995]

Implemented the remaining findings from Nir's review

1. Issue: executable dependency-source commands were missed in shell context. Commands behind temporary assignment prefixes, inline then/do/else, brace groups, and package-manager surfaces could escape detection.
   Solution: normalize executable command segments and control wrappers before matching, and strip valid temporary assignment words consistently across npm, Yarn, pip, Poetry, and Maven detection.

2. Issue: shell-state tracking mishandled compact/invalid assignments. One-line case assignments or function calls could be ignored, whitespace-separated NAME = value could be treated as an assignment, and export NAME=value OTHER_NAME could lose the valid assignment.
   Solution: recognize only valid shell assignment-word syntax, conservatively mark one-line case results as unresolved, retain valid exported assignments with trailing names, and allow a later definite canonical assignment to clear earlier ambiguity.

3. Issue: word-level heredoc quoting was incomplete. Mixed quoted or partially escaped delimiters could hide generated configuration or cause unrelated body text to be analyzed.
   Solution: add bounded static quote removal for mixed quoted, escaped, and unquoted delimiter segments, while ensuring malformed/unsupported delimiter words do not consume later executable commands.

Validation: 85 dependency-source tests, 224 focused dependency/report/meta tests, 8 terminal/JSON/Markdown/SARIF end-to-end cases, and 2,316 repository tests passed; Ruff, focused mypy, and git diff --check also passed.

[mohgupta-ship-it]

Powered by Codex: PR council review result.

This is a triage signal, not a maintainer approval.

• Rating: new feature
• Confidence: medium-low
• Status read: No review, green checks, conflicting
• Review method: fresh GitHub metadata/body/files/reviews/checks plus selected diffs; council lenses were spec fit, dead-code/reachability, YAGNI/scope, design/coupling, and code standards/tests.
• Council assessment: Large deterministic dependency-source parser and SC10 policy path. Broad tests exist, but false-positive policy and authoritative HIGH behavior need owner review.
• Recommended action: Require Mohit/security-owner review before marking ready; resolve conflict.

[rng1995]

[SkillSpector Review]

Manual Review Needed at 6b84b11. The authenticated reviewer (rng1995) is also this PR's author, so an independent approval or changes-requested decision cannot be supplied reliably or accepted as the required review. The security-sensitive parser is also a large change (more than 4,000 added lines including tests) and currently conflicts with main. Please rebase it and obtain an independent maintainer/security review of the resulting current-head diff.

[rng1995]

Rebased this PR onto current main (92e8e65) and resolved the analyzer/changelog conflicts in 5dd64ef. The integration preserves current-main fallback enrichment for ordinary findings while keeping authoritative SC9/SC10 records canonical and fail-closed.

Validation on the rebased head:

• 5,739 passed, 14 skipped, 4 xfailed (full non-provider/non-integration suite)
• 327 focused dependency/report/graph tests passed
• Ruff lint and format checks passed
• focused mypy passed
• git diff --check passed

GitHub now reports the PR mergeable; fresh exact-head CI is running. The remaining requested gate is independent maintainer/security review.

[rng1995]

[SkillSpector Review]

Manual Review Needed at exact head 983105b2b062ba0f8827c7c1d15deb8337b5af45.

The authenticated reviewer (rng1995) is also this PR's author, so I cannot provide the independent approval or changes-requested decision required for this security-sensitive dependency-source parser. The head has materially changed since the prior marked review and is now mergeable, but GitHub reports UNSTABLE and no exact-head checks are attached.

Please obtain an independent maintainer/security review of the complete current diff and a green exact-head CI run before merging. No self-approval is being recorded.

[rng1995]

[SkillSpector Review]

Manual review needed on exact head 20c39f2abfb2c11dde744125b4ef4b65733c7541. The authenticated reviewer (rng1995) is also this PR's author, so I cannot provide an independent approval or changes-requested decision. I reviewed the complete current diff, all synchronization commits through #575, prior discussion, and the absence of exact-head hosted checks. The finding-related PR blobs remained unchanged across the final synchronization.

Two required security/correctness fixes remain:

1. src/skillspector/dependency_sources.py:2000-2021 applies _literal_assignments(content)—a shell-state model—to direct configuration files. For example, a .npmrc containing SOURCE=https://registry.npmjs.org/ followed by registry=${SOURCE} is resolved to the canonical registry and produces no SC10 finding. npm expands ${SOURCE} from the process environment, not from another .npmrc key, so an attacker-controlled environment can redirect the runtime registry while the scan reports it canonical. Pass empty or ecosystem-native interpolation state to direct config parsers; retain shell assignment resolution only for actual shell/generated expanding heredocs, and add a direct-config variable-shadowing regression.

2. src/skillspector/nodes/report.py:129-132 sanitizes only top-level string values in Finding.evidence. Nested dictionaries/lists are supported by the model but are returned unchanged, allowing credential-bearing URLs in nested evidence to leak into terminal, JSON, Markdown, and SARIF output. Recursively sanitize string leaves in supported evidence containers while preserving non-string scalar types, and add nested dict/list regressions across all output formats.

No exact-head hosted checks are attached, GitHub reports UNSTABLE, and an independent maintainer/security review remains required after these fixes.

[yashrajp22]

These comments are based on inspecting the current PR code. The local package scans ran on main (1c0eb56), so they do not verify this PR branch. Please address the inline findings before merging.

[rng1995]

Review remediation is complete at commit fbba4c4174883d9b0ff7a961159c898c8209262b, including the nested-evidence fix in 4f64478.

Yashraj's five inline findings are addressed:

• Same-line assignments or function calls cannot reuse an older canonical registry value; uncertain ordering remains unresolved.
• Markdown prose assignments cannot supply values to executable shell fences.
• Unexpected TOML tool shapes are handled locally, preserving findings already collected by other supply-chain checks.
• SC10 uses the remaining workflow time and finding allowance during parsing, preserves collected findings, and records partial coverage when either limit stops inspection.
• Generated Poetry, Maven, and Cargo configuration resolves variables at their original script positions before canonical filtering.

The additional review-summary findings are also addressed: direct configuration files no longer borrow shell assignment state, and credential redaction recursively covers nested evidence keys/string values in dictionaries, lists, and tuples across terminal, JSON, Markdown, and SARIF reports while preserving scalar types.

Validation: 6,327 unit tests passed (14 skipped, 4 expected failures); 336 focused parser tests and 77 offline integration tests passed. Ruff lint/format, focused mypy, and diff checks passed. Combining all four PRs (#383, #551, #555, #556) produced no merge conflicts; 1,218 combined regression tests and 77 combined offline integration tests passed.

All five inline review threads are resolved, and GitHub currently reports this head as mergeable. This clears the conflict raised in Mohit's council comment. Mohit's requested security-owner review of the false-positive policy and authoritative HIGH behavior remains a human review gate; these fixes do not supply that approval. At the latest check, hosted lint, TypeScript, DCO, and Docker smoke checks passed, while the hosted unit-test job was still running. This summary is not an independent approval; no merge was performed.