Skip to content

refactor(prover): remove legacy unused reachability constraints#2441

Open
eviehoward wants to merge 3 commits into
NVIDIA:mainfrom
eviehoward:2412-remove-legacy-reachability-constraints/eviehoward
Open

refactor(prover): remove legacy unused reachability constraints#2441
eviehoward wants to merge 3 commits into
NVIDIA:mainfrom
eviehoward:2412-remove-legacy-reachability-constraints/eviehoward

Conversation

@eviehoward

@eviehoward eviehoward commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Removes can_write_to_endpoint() and the solver variables and encoding methods that exist solely to support it. The active query layer uses can_exfil_via_endpoint() and direct credential lookups exclusively; the removed Z3 encodings were scaffolding for query categories that were never implemented or were later cut. Confirmed zero callers across the monorepo; crate is not published externally and no release notes reference its public API surface.

Also removes three dead write-capability helpers from registry.rs that lost their only callers as a direct consequence of the model.rs cleanup.

Related Issue

Closes #2412

Changes

  • Removed can_write_to_endpoint() — zero callers confirmed across the monorepo
  • Removed binary_can_write, credential_has_write — only consumed by can_write_to_endpoint()
  • Removed credential_has_destructive, filesystem_readable — directly dead, previously silenced with #[allow(dead_code)]
  • Removed encode_credentials(), encode_filesystem() — exclusively populated the above fields
  • Removed two #[allow(dead_code)] allowances, HashMap::new() initialisers, and the encode_credentials() / encode_filesystem() calls inside build().
  • Removed BinaryProtocol::can_write(), BinaryCapability::can_write() — made dead by the model.rs cleanup above
  • Removed write_mechanisms() — pre-existing dead code, no callers prior to this series

Follow-up Dead Code

This PR leaves behind two dead code clusters that are out of scope here but should be tracked:

  1. registry.rs - write action data layer (caused by this PR). Removing can_write() and write_mechanisms() orphaned their only consumers of BinaryProtocol::actions, BinaryAction, and ActionType::Write/Destructive. The data is still loaded from YAML at parse time but never queried.
  2. credentials.rs - API registry subsystem (pre-existing, surfaced by this PR). write_actions_for_scopes and destructive_actions_for_scopes were killed here (their sole caller encode_credentials() was removed). The cascade beneath them - actions_for_scopes, ApiCapability, ApiAction, and all associated loaders and serde types - was already dead before this PR. This is the full API registry subsystem in credentials.rs, which was never fully wired up.

To avoid scope creep I have omitted these from this PR but can address it here if preferred - left as a follow-up outside scope of this PR for now.

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
  • E2E tests added/updated (if applicable)

cargo test -p openshell-prover passes. No new tests added — existing tests (test_findings_for_github_policy, test_wildcard_endpoint_covering_credential_host_emits_credential_reach, test_known_metadata_hostname_emits_link_local_finding, test_empty_policy_no_findings) cover all four active finding categories end-to-end. can_write_to_endpoint() was never invoked by prove() or any live query path — no test could have detected its presence or absence.

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
  • Architecture docs updated (if applicable) — no architecture doc covers prover model internals

Remove can_write_to_endpoint() and the solver variables and encoding
methods that exist solely to support it. No active query calls this
method; confirmed zero callers across the monorepo. Crate is not
published externally, so no public API concern.

Removed as root cause:
- can_write_to_endpoint(): zero callers in the codebase

Removed as direct consumers of can_write_to_endpoint():
- binary_can_write: only read inside can_write_to_endpoint()
- credential_has_write: only read inside can_write_to_endpoint()

Removed as directly dead (no query ever read them):
- credential_has_destructive: flagged by compiler, silenced with
  #[allow(dead_code)] rather than removed
- filesystem_readable: flagged by compiler, silenced with
  #[allow(dead_code)] rather than removed

Removed as now-empty encoding methods:
- encode_credentials(): exclusively populated credential_has_write
  and credential_has_destructive
- encode_filesystem(): exclusively populated filesystem_readable

Also removed two #[allow(dead_code)] allowances and the corresponding
HashMap::new() initialisers and build() call sites.

Active query behavior is unchanged. All existing prover tests pass
without modification and cover all four finding categories end-to-end.
No new tests added as they would duplicate existing coverage exactly.

Note: removing encode_credentials() leaves write_actions_for_scopes()
and destructive_actions_for_scopes() in credentials.rs with no callers.
Left as a follow-up outside the scope of this change.

Closes NVIDIA#2412

Signed-off-by: Evie Howard <evhoward@redhat.com>
…gistry

BinaryProtocol::can_write(), BinaryCapability::can_write(), and
write_mechanisms() lost their only callers when can_write_to_endpoint()
and its supporting model fields were removed. Delete the now-dead methods.

write_mechanisms() had no callers prior to this series; the two can_write()
methods were made dead by the preceding model.rs cleanup.

Signed-off-by: Evie Howard <evhoward@redhat.com>
The module doc incorrectly listed write-bypass violation detection as a
prover capability. The prover only checks reachability queries; write-bypass
detection was never implemented.

Signed-off-by: Evie Howard <evhoward@redhat.com>
@copy-pr-bot

copy-pr-bot Bot commented Jul 23, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor(prover): remove legacy unused reachability constraints

1 participant