Close Rust advisory owner-chain gaps - #197
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughRustSec 예외 목록을 축소하고 Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(240,240,255,0.5)
actor CI as CI/Developer
end
rect rgba(255,240,240,0.5)
participant Script as verify_supply_chain.py
participant Lock as Cargo.lock
participant Audit as apps/.../.cargo/audit.toml
participant Tests as services/.../test_supply_chain_policy.py
participant Docs as docs/security/dependency-policy.md
end
CI->>Script: 검증 실행
Script->>Lock: 읽기 (패키지 그래프)
Script->>Script: 그래프 분석 (ancestors, reachability, 경로 매칭)
Script->>Audit: 읽기 (예외 목록)
Script->>Docs: 검증(문서화된 예외 범위)
Script->>Tests: 위반/허용 결과 제공
Script->>CI: 결과 리포트 (통과/실패)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 2/5 reviews remaining, refill in 27 minutes and 36 seconds. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 683-688: The current check only gathers direct owners via
cargo_lock_dependency_owners for RUST_GLIB_LEGACY_EXCEPTION_PACKAGE, which lets
a non-Tauri root bypass the exception if it shares intermediate package keys;
change the logic to compute all ancestor/root owners by traversing reverse
dependencies starting from the glib package (use/replace
cargo_lock_reachable_package_keys_by_name and cargo_lock_dependency_owners
usage) so you collect the full ancestor set for
RUST_GLIB_LEGACY_EXCEPTION_PACKAGE and then validate that every ancestor/root is
inside the documented Tauri GTK chain (i.e., the allowlist represented by
RUST_GLIB_LEGACY_ROOT_NAME and related GTK chain names); apply the same
replacement to the analogous block around lines 792-801.
In `@services/analysis-engine/tests/test_supply_chain_policy.py`:
- Around line 992-1000: Update the
test_tauri_main_capability_uses_explicit_core_permissions test to also assert
that emit permissions are absent: after reading capability into content
(variable capability / content), add negative assertions ensuring
'"core:event:allow-emit"' and '"core:event:allow-emit-to"' are not in content so
the test fails if emit permissions are reintroduced; keep the existing checks
for core:default removal and listen/unlisten presence.
🪄 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: 52aa9536-1be9-472b-995f-898c98f1ef72
⛔ Files ignored due to path filters (5)
apps/desktop/src-tauri/Cargo.lockis excluded by!**/*.lockapps/desktop/src-tauri/gen/schemas/acl-manifests.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/capabilities.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/desktop-schema.jsonis excluded by!**/gen/**apps/desktop/src-tauri/gen/schemas/macOS-schema.jsonis excluded by!**/gen/**
📒 Files selected for processing (5)
apps/desktop/src-tauri/.cargo/audit.tomlapps/desktop/src-tauri/capabilities/main.jsondocs/security/dependency-policy.mdscripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 816-821: The current boolean check only compares ancestor name
sets and allows a bypass like bandscope-desktop -> gtk -> glib when another
valid tauri chain exists; change the logic that uses legacy_glib_ancestors and
ancestor_names so it verifies per root-to-glib path inclusion of the required
chain (not just set membership). Concretely, for each candidate root in
allowed_app_roots (and for legacy_glib_ancestors entries), enumerate or traverse
the dependency paths from that root to glib and ensure at least one path
contains the documented chain elements (e.g., tauri -> tauri-runtime-wry -> wry
-> webkit2gtk -> gtk -> glib) before allowing the exception; reject if any root
can reach glib by a path that does not include the required chain. Update the
code around legacy_glib_ancestors, glib_exception_owned_packages,
allowed_app_roots, and ancestor_names to implement this path-level check and add
a regression test reproducing the described lockfile scenario (bandscope-desktop
-> gtk -> glib plus the normal tauri chain) to assert the function now returns
False.
🪄 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: 64e33a4d-174b-468c-813d-588fcd5c9835
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 803-831: The function glib_legacy_exception_owners_are_allowed
currently only checks ancestor name subsets and allowed app roots, which permits
short-circuit paths like "tauri -> glib 0.18.5"; fix it by also extracting the
direct owner names (the immediate owner component, e.g. ancestor.rsplit(" ",
1)[0] or otherwise derived from legacy_glib_ancestors/package_dependencies) and
enforce that this set is a subset of RUST_GLIB_LEGACY_DIRECT_OWNER_NAMES before
returning true; update glib_legacy_exception_owners_are_allowed to perform this
extra subset check (in addition to the existing ancestor_names <=
RUST_GLIB_LEGACY_ALLOWED_ANCESTOR_NAMES |
RUST_GLIB_LEGACY_ALLOWED_APP_ROOT_NAMES and the existing
glib_allowed_app_roots_reach_glib_through_tauri check), and add a regression
test in services/analysis-engine/tests/test_supply_chain_policy.py that asserts
a path like "tauri -> glib 0.18.5" is rejected.
🪄 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: 4e463b7f-3247-411e-a595-a3342cc27cc6
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 811-845: The current glib_legacy_exception_owners_are_allowed only
validates name sets and allows shortened chains (e.g., bandscope-desktop ->
tauri -> gtk -> glib); update the logic so each allowed app root has at least
one dependency path in package_dependencies that contains the full ordered
intermediary chain (tauri -> tauri-runtime-wry -> wry -> webkit2gtk -> gtk ->
glib with the expected versions) instead of just checking set membership.
Concretely, change or extend glib_allowed_app_roots_reach_glib_through_tauri to
traverse package_dependencies from each allowed_app_root and assert an ordered
subsequence of nodes (using package names via rsplit(" ",1)[0]) exists before
reaching the glib package, and add a regression test that builds the
bandscope-desktop-like graph (app -> tauri -> gtk -> glib) to assert it fails
while a full chain passes.
🪄 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: bbe9ff8b-332d-4562-9cbb-6c0e01d1d0e4
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
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 845-852: The current logic only enforces the full chain when
allowed_app_roots is non-empty via
glib_allowed_app_roots_reach_glib_through_tauri(package_dependencies,
allowed_app_roots), which lets abbreviated chains like tauri->gtk->glib pass
when allowed_app_roots (off_chain_ancestors) is empty; always validate that any
detected Tauri ancestor enforces RUST_GLIB_LEGACY_EXPECTED_CHAIN_NAMES
regardless of allowed_app_roots by adding a check that ancestor_names (or the
ancestor set computed from off_chain_ancestors/ancestor_names) is a superset of
RUST_GLIB_LEGACY_EXPECTED_CHAIN_NAMES when a Tauri ancestor is present, in
addition to the existing allowed-app-root checks (keep existing
glib_allowed_app_roots_reach_glib_through_tauri and the final ancestor_names <=
... check).
🪄 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: 7c0c480b-e5c3-45bd-946e-c8cf2a24a0c5
📒 Files selected for processing (2)
scripts/checks/verify_supply_chain.pyservices/analysis-engine/tests/test_supply_chain_policy.py
Summary
rand 0.7.3,fxhash,kuchikiki,selectors,phf 0.8, andproc-macro-hackadvisory/deprecation path.glib 0.18.5owner-chain exception guarded by supply-chain policy tests.core:defaultto explicit event permissions.Closes part of #194.
Follow-up tracker for the remaining upstream-owned glib exception: #196.
Verification
Earlier full local verification on this change set also passed:
Security Notes
glib 0.18.5advisory is allowed only for the externally owned Tauri/wry/webkit2gtk/gtk GTK3 dependency chain.verify_supply_chain.pynow fails closed for stalerand 0.7.x, stalefxhashexceptions, non-numericglibversions, unownedglib 0.18.5, mixed owners, and unexpected Tauri-reachable owners.