fix: patch rust dependency advisory ownership - #191
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughCargo 감사 무시 항목을 갱신하고 보안 의존성 정책 문서를 보강했으며, Changes
Sequence Diagram(s)sequenceDiagram
participant CI as CI / Caller
participant Script as verify_supply_chain.py
participant Lockfile as apps/desktop/src-tauri/Cargo.lock
participant Reporter as Exit/Reporter
CI->>Script: 실행
Script->>Lockfile: 읽기/파싱 (package, version, dependencies)
Script->>Script: 검증 로직 실행\n(yanked fastrand, rand 버전 비교, 소유자 체인 확인)
Script->>Reporter: 위반 목록 반환 및 종료 코드 설정
Reporter-->>CI: 성공/실패 상태
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 656-663: The function rust_dependency_advisory_violations
currently calls lockfile.read_text() which will raise FileNotFoundError if the
Cargo.lock is missing; change the logic in rust_dependency_advisory_violations
to detect a missing lockfile (via lockfile.exists() or wrapping read_text() in
try/except FileNotFoundError), append a descriptive violation entry to the
violations list (e.g., "Cargo.lock missing: <path>") and return that list
instead of letting the exception propagate; keep the rest of the parsing logic
unchanged so the function continues to aggregate violations when the file is
present.
- Around line 680-681: The current exception only checks version equality
(version == RUST_RAND_LEGACY_EXCEPTION_VERSION) and should additionally verify
the ownership/dependency chain before skipping; update the conditional around
that line to inspect the package ownership chain (e.g., walk the
dependency/owners list for the package entry) and only continue when the chain
exactly matches the documented path selectors -> phf_codegen -> phf_generator ->
rand (or the canonical list of owner package names you derive from Cargo.lock),
otherwise treat it as non-exempt and run the normal verification; ensure you
reference RUST_RAND_LEGACY_EXCEPTION_VERSION and the local package's
owners/ownership_chain variable or function used in this file, and add a clear
log or error when a version match exists but the ownership chain does not.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e2ccc79e-7bef-4762-85ef-25fc31da9f0b
⛔ Files ignored due to path filters (1)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/desktop/src-tauri/.cargo/audit.tomldocs/security/dependency-policy.mdscripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
scripts/checks/verify_supply_chain.py (1)
672-700:⚠️ Potential issue | 🟠 Major | ⚡ Quick win레거시 예외를 전역 불리언으로 처리하면 비인가
rand 0.7.3도 함께 통과됩니다Line 672-696에서
legacy_exception_allowed를 한 번만 계산해 모든rand 0.7.3항목에 재사용하고 있습니다. 허용 체인이 하나만 존재해도 다른 owner 경로의rand 0.7.3까지 면제될 수 있어 정책 우회가 가능합니다.🔧 제안 패치
def rust_dependency_advisory_violations( lockfile: Path = Path("apps/desktop/src-tauri/Cargo.lock"), ) -> list[str]: @@ package_dependencies = cargo_lock_package_dependencies(lockfile) - legacy_exception_allowed = cargo_lock_has_dependency_chain( + legacy_exception_allowed = cargo_lock_has_dependency_chain( package_dependencies, RUST_RAND_LEGACY_EXCEPTION_CHAIN ) + expected_legacy_owner = RUST_RAND_LEGACY_EXCEPTION_CHAIN[-2] + legacy_rand_owners = cargo_lock_dependency_owners( + package_dependencies, RUST_RAND_LEGACY_EXCEPTION_CHAIN[-1] + ) @@ if version == RUST_RAND_LEGACY_EXCEPTION_VERSION: - if legacy_exception_allowed: + if legacy_exception_allowed and legacy_rand_owners == {expected_legacy_owner}: continue violations.append( f"{lockfile}: rand {version} matches the legacy exception version " "but does not have the documented Tauri/kuchikiki owner chain " f"for {RUST_RAND_ADVISORY_ID}" ) continue @@ +def cargo_lock_dependency_owners( + package_dependencies: dict[str, list[str]], dependency: str +) -> set[str]: + """Return package keys that directly reference the target dependency key.""" + owners: set[str] = set() + for owner, dependency_tokens in package_dependencies.items(): + if dependency in dependency_tokens: + owners.add(owner) + return ownersBased on learnings: Apply dependency, SBOM, and supply-chain rules from
docs/security/dependency-policy.mdto dependency additions, GitHub Actions, releases, bundled binaries, and model artifacts.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/checks/verify_supply_chain.py` around lines 672 - 700, The code computes legacy_exception_allowed once and reuses it for all rand 0.7.3 entries, allowing one valid owner chain to exempt other unrelated rand entries; to fix, move the call to cargo_lock_has_dependency_chain(...) inside the loop where you detect current_name == "rand" and version == RUST_RAND_LEGACY_EXCEPTION_VERSION so you compute legacy_exception_allowed per found rand package (or otherwise compute/check the owner chain per-package) before deciding to append to violations; reference symbols: legacy_exception_allowed, cargo_lock_has_dependency_chain, current_name, version, RUST_RAND_LEGACY_EXCEPTION_VERSION, RUST_RAND_LEGACY_EXCEPTION_CHAIN, and violations.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 767-769: The current fallback using dependency_name allows matches
that ignore the version (e.g., matching "rand" when the exception specified
"rand 0.7.3"); update the check in the function that computes dependency_tokens
(uses variables dependency_tokens, dependency_name, dependency) to require an
exact token match including the version so only dependency in dependency_tokens
is accepted (remove the "or dependency_name in dependency_tokens" fallback) and
ensure tokens are normalized (trimmed) before comparison so whitespace
differences don't bypass the strict match.
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 395-420: Add a regression test that covers the “mixed owner” case
for the legacy rand 0.7.3 exception: create a new test (e.g.,
test_supply_chain_check_rejects_mixed_owner_legacy_rust_rand_exception)
alongside test_supply_chain_check_rejects_unowned_legacy_rust_rand_exception
that loads verify_supply_chain_rust_rand_unowned and writes a Cargo.lock
containing two [[package]] entries for rand 0.7.3 — one matching the documented
Tauri/kuchikiki owner checksum and another entry or package referencing the same
version but with a different owner/checksum — then call
rust_dependency_advisory_violations(lockfile) and assert the violation message
flags rand 0.7.3 as invalid (i.e., not allowed when a mixed owner chain exists).
---
Duplicate comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 672-700: The code computes legacy_exception_allowed once and
reuses it for all rand 0.7.3 entries, allowing one valid owner chain to exempt
other unrelated rand entries; to fix, move the call to
cargo_lock_has_dependency_chain(...) inside the loop where you detect
current_name == "rand" and version == RUST_RAND_LEGACY_EXCEPTION_VERSION so you
compute legacy_exception_allowed per found rand package (or otherwise
compute/check the owner chain per-package) before deciding to append to
violations; reference symbols: legacy_exception_allowed,
cargo_lock_has_dependency_chain, current_name, version,
RUST_RAND_LEGACY_EXCEPTION_VERSION, RUST_RAND_LEGACY_EXCEPTION_CHAIN, and
violations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 24a1f262-dd76-41d6-b95b-d5bca3b3de9e
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 707-710: The current direct int(part) conversion for parts can
raise ValueError on non-numeric prerelease segments (e.g., "0.9.3-alpha.1");
change the parsing around version -> parts to be defensive: iterate the first
three segments, try to int(part) in a try/except, on ValueError record a
validation/violation entry for this version (so the failure is reported) and
skip the patched-version comparison for this entry (treat it as unpatched),
otherwise collect parsed ints and pad to length 3 with zeros, then compute
rand_series and lookup RUST_RAND_PATCHED_VERSIONS as before (use the existing
rand_series, patched_version, parts names).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 48ae1c5e-781f-4971-beb2-f8c150122d18
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 664-680: The current rust_dependency_advisory_violations reads
Cargo.lock as plain text and assumes ordering/format (e.g. name before version
and only multiline dependencies arrays), which breaks on valid TOML forms
(inline arrays, different key order) and causes missed advisories; instead,
update rust_dependency_advisory_violations to load and iterate the parsed TOML
package tables (use a TOML parser instead of manual line parsing) and then use
cargo_lock_package_dependencies, cargo_lock_has_dependency_chain and
cargo_lock_dependency_owners against those parsed package entries (ensuring
inline "dependencies" arrays and any key order are handled), so the owner-chain
and vulnerability checks operate on canonical package objects rather than raw
text.
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 343-579: Add Cargo.lock fixtures that exercise format variations:
for tests using rust_dependency_advisory_violations (e.g.,
test_supply_chain_check_rejects_vulnerable_rust_rand_lockfile and
test_supply_chain_check_rejects_unowned_legacy_rust_rand_exception), include one
[[package]] block where the version field appears before the name field and
another package entry that uses an inline dependencies = ["rand 0.7.3"] form
(not multiline). Update the lockfile.write_text payloads in those tests (or add
two small new tests) to include these variants so the parser is exercised
against both "version-first" ordering and inline dependencies formats. Ensure
the added entries still trigger the same asserted violations so existing
assertions remain valid.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5e84b923-a504-4d1c-a336-67ba5ee145a7
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 55-59: The RUST_RAND_PATCHED_VERSIONS mapping misses the 0.7
series so non-0.7.3 versions slip past the check; add an entry for (0, 7): (0,
7, 3) to RUST_RAND_PATCHED_VERSIONS so only 0.7.3 is treated as the allowed
legacy exception, leaving other 0.7.x versions to result in patched_version
being None (and thus flagged); update the RUST_RAND_PATCHED_VERSIONS constant in
verify_supply_chain.py accordingly and ensure any logic that interprets
patched_version still treats None as a violation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9520d44d-f0d7-4818-b39b-4c63b0c81499
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
seonghobae
enabled auto-merge
April 30, 2026 09:36
Collaborator
Author
|
@coderabbitai full review |
Contributor
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@scripts/checks/verify_supply_chain.py`:
- Around line 853-860: The function cargo_lock_has_dependency_chain uses
zip(package_chain, package_chain[1:]) which silently truncates mismatched
lengths and triggers a Ruff warning; replace that zip usage with
itertools.pairwise(package_chain) inside cargo_lock_has_dependency_chain and add
the necessary import for itertools.pairwise so the all(...) comprehension
iterates adjacent pairs explicitly (i.e., call
cargo_dependency_targets_package(owner, dependency) for each pair from
pairwise(package_chain)).
- Around line 699-723: The code currently accepts versions with more than three
dot-separated segments by truncating to the first three; update the validation
so that after computing segments = version.split(".") you explicitly reject any
version with len(segments) > 3 by appending a violations entry (using the same
message style and referencing RUST_RAND_ADVISORY_ID and the lockfile/version)
and continue; keep the existing numeric parsing of parsed_parts for 1–3 segment
cases and only proceed to compute rand_series/patch lookup when there are
exactly 1–3 segments (and no non-numeric segments).
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 540-564: 현재 rust_dependency_advisory_violations( )가 비수치 세그먼트만 검사하고
4세그먼트 숫자(예: 0.8.6.1)를 허용해 회귀를 발생시킬 수 있으므로, verify_supply_chain.py의
rust_dependency_advisory_violations 함수에서 버전 파싱 로직을 강화해 각 세그먼트를 점(.)으로 분리한 뒤
3세그먼트(major.minor.patch)를 초과하는 숫자 세그먼트가 있거나 어떤 세그먼트라도 숫자가 아닌 경우 모두 위반으로 처리하도록
수정하세요; 위반 메시지는 기존 형식(f"{lockfile}: rand 0.9.3-alpha.1 ...")을 유지하되 추가 케이스(예:
"rand 0.8.6.1 has a non-standard/extra numeric version segment ...")도 포함되게
작성하세요.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1791aa36-f086-4481-ae63-b600179fb0eb
⛔ Files ignored due to path filters (1)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
apps/desktop/src-tauri/.cargo/audit.tomldocs/security/dependency-policy.mdscripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Collaborator
Author
|
@coderabbitai resolve |
Contributor
✅ Actions performedComments resolved and changes approved. |
This was referenced Apr 30, 2026
This was referenced Apr 30, 2026
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
randadvisory by moving the repo-controlledrand 0.8line from0.8.5to0.8.6.fastrand 2.4.0transitive crate to2.4.1.rand0.8/0.9/0.10vulnerable ranges and yankedfastrand 2.4.0cannot silently return.rand 0.7.3andproc-macro-hackTauri/kuchikiki owner chains with a narrow Cargo audit allowance.Closes #188
Part of #189
Verification
uv run --project services/analysis-engine pytest services/analysis-engine/tests/test_supply_chain_policy.py→ 38 passedpython3 scripts/checks/verify_supply_chain.py→ passedpython3 scripts/checks/security_gates.py→ passedcargo metadata --locked --format-version 1→ passedcargo audit→ passedcargo test --manifest-path apps/desktop/src-tauri/Cargo.toml --locked→ 8 passedBANDSCOPE_ENABLE_RUST_CHECK=1 ./scripts/harness/quickcheck.sh→ passedSecurity Notes
rand 0.8advisory is fixed by lockfile update; yankedfastrand 2.4.0is removed from the lockfile.rand 0.7.3advisory is narrowly allowed inapps/desktop/src-tauri/.cargo/audit.tomlbecause it is externally owned throughtauri/tauri-build→tauri-utils→kuchikiki→selectors→phf_codegen→phf_generator, with no compatible lockfile-only update path found.services/analysis-engine/tests/test_supply_chain_policy.pykeep the allowance narrow by blocking vulnerablerand 0.8,0.9, and0.10ranges and yankedfastrand 2.4.0.