-
Notifications
You must be signed in to change notification settings - Fork 0
feat(impact): ship deterministic commandf impact #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
a6805a6
ca25db3
6431dfd
f4311ff
b350c73
e6b7972
d3b8ca4
989e387
6d8e22b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| name: cf12-impact-proof | ||
|
|
||
| on: | ||
| pull_request: | ||
| paths: | ||
| - .github/workflows/cf12-impact-proof.yml | ||
| - Cargo.toml | ||
| - Cargo.lock | ||
| - crates/commandf-pkg/** | ||
| - crates/commandf-cli/** | ||
| - specs/013-cf-12-impact/** | ||
| push: | ||
| branches: | ||
| - impl/cf12-impact-cli | ||
| paths: | ||
| - .github/workflows/cf12-impact-proof.yml | ||
| - Cargo.toml | ||
| - Cargo.lock | ||
| - crates/commandf-pkg/** | ||
| - crates/commandf-cli/** | ||
| - specs/013-cf-12-impact/** | ||
| workflow_dispatch: | ||
|
|
||
| permissions: | ||
| contents: read | ||
|
|
||
| env: | ||
| CF12_PROOF_CONTAINER: docker.io/library/rust@sha256:9146b0f62e1939989aa96fc8d89699a43c5635bf212819235a773e1a9e71a98f | ||
|
|
||
| jobs: | ||
| deterministic-impact-cli: | ||
| runs-on: ubuntu-24.04 | ||
| container: | ||
| image: docker.io/library/rust@sha256:9146b0f62e1939989aa96fc8d89699a43c5635bf212819235a773e1a9e71a98f | ||
| timeout-minutes: 15 | ||
| steps: | ||
| - uses: actions/checkout@fbc6f3992d24b796d5a048ff273f7fcc4a7b6c09 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Assert pinned execution toolchain | ||
| run: | | ||
| set -euo pipefail | ||
| rustc --version --verbose | ||
| cargo --version | ||
| test "$(rustc --version | awk '{print $2}')" = "1.97.1" | ||
|
|
||
| - name: Prove byte-identical commandf impact output | ||
| run: | | ||
| set -euo pipefail | ||
| cargo test --locked -p commandf --test impact_determinism_proof -- --nocapture \ | ||
| | tee /tmp/cf12-impact-proof.log | ||
| grep -oE 'CF12_IMPACT_SHA256=[0-9a-f]{64}' /tmp/cf12-impact-proof.log \ | ||
| | tail -n 1 \ | ||
| | tee /tmp/cf12-impact.sha256 | ||
| test -s /tmp/cf12-impact.sha256 | ||
|
|
||
| - name: Assert repository remains clean | ||
| run: | | ||
| set -euo pipefail | ||
| status="$(git -c safe.directory="$GITHUB_WORKSPACE" status --porcelain)" | ||
| test -z "$status" | ||
|
|
||
| - name: Upload impact determinism evidence | ||
| uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 | ||
| with: | ||
| name: cf12-impact-proof | ||
| path: /tmp/cf12-impact.sha256 | ||
| if-no-files-found: error | ||
| retention-days: 3 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,90 @@ | ||
| use std::fs; | ||
| use std::io; | ||
| use std::path::PathBuf; | ||
|
|
||
| use commandf_pkg::{ | ||
| build_context_graph, build_impact_report, diff_package_archives, LockedPackage, Lockfile, | ||
| PackageCache, PackageName, | ||
| }; | ||
|
|
||
| pub fn run( | ||
| package: String, | ||
| before_lock: PathBuf, | ||
| before_cache: PathBuf, | ||
| after_lock: PathBuf, | ||
| after_cache: PathBuf, | ||
| ) -> Result<Vec<u8>, Box<dyn std::error::Error>> { | ||
| let package_name = PackageName::parse(package)?; | ||
| let before_lockfile = Lockfile::from_slice(&fs::read(before_lock)?)?; | ||
| let after_lockfile = Lockfile::from_slice(&fs::read(after_lock)?)?; | ||
| require_lock_v2(&before_lockfile, "before")?; | ||
| require_lock_v2(&after_lockfile, "after")?; | ||
| let before_locked = select_locked_package(&before_lockfile, package_name.as_str())?; | ||
| let after_locked = select_locked_package(&after_lockfile, package_name.as_str())?; | ||
|
|
||
| let before_cache = PackageCache::new(before_cache); | ||
| let after_cache = PackageCache::new(after_cache); | ||
| before_cache.verify(&before_locked.sha256)?; | ||
| after_cache.verify(&after_locked.sha256)?; | ||
|
|
||
| let before_bytes = read_locked_archive(&before_cache, before_locked)?; | ||
|
Comment on lines
+27
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2. Cache verification race The selected archive is verified, discarded, and then reopened with plain fs::read, so a cache object changed between those operations can supply bytes that do not match the lock digest while the diff still labels them with that digest. This breaks the verified-cache trust boundary and can produce an internally inconsistent impact report. Agent Prompt
|
||
| let after_bytes = read_locked_archive(&after_cache, after_locked)?; | ||
| let diff = diff_package_archives( | ||
| package_name.to_string(), | ||
| &before_locked.version, | ||
| &before_locked.sha256, | ||
| &before_bytes, | ||
| &after_locked.version, | ||
| &after_locked.sha256, | ||
| &after_bytes, | ||
| )?; | ||
| let before_graph = build_context_graph(&before_lockfile, &before_cache)?; | ||
| let after_graph = build_context_graph(&after_lockfile, &after_cache)?; | ||
| let report = build_impact_report(&diff, &before_graph, &after_graph)?; | ||
| Ok(report.to_json_bytes()?) | ||
| } | ||
|
|
||
| fn require_lock_v2(lockfile: &Lockfile, side: &'static str) -> io::Result<()> { | ||
| if lockfile.schema == Lockfile::SCHEMA_V2 { | ||
| return Ok(()); | ||
| } | ||
| Err(io::Error::new( | ||
| io::ErrorKind::InvalidData, | ||
| format!( | ||
| "impact requires commandf.lock schema 2 on {side}; found schema {}", | ||
| lockfile.schema | ||
| ), | ||
| )) | ||
| } | ||
|
|
||
| fn read_locked_archive(cache: &PackageCache, locked: &LockedPackage) -> io::Result<Vec<u8>> { | ||
| fs::read( | ||
| cache | ||
| .root() | ||
| .join("sha256") | ||
| .join(format!("{}.tgz", locked.sha256)), | ||
| ) | ||
| } | ||
|
|
||
| fn select_locked_package<'a>( | ||
| lockfile: &'a Lockfile, | ||
| package_name: &str, | ||
| ) -> Result<&'a LockedPackage, io::Error> { | ||
| let mut matches = lockfile | ||
| .packages | ||
| .iter() | ||
| .filter(|candidate| candidate.name == package_name); | ||
| let selected = matches.next().ok_or_else(|| { | ||
| io::Error::new( | ||
| io::ErrorKind::NotFound, | ||
| format!("package {package_name} is not present in the lockfile"), | ||
| ) | ||
| })?; | ||
| if matches.next().is_some() { | ||
| return Err(io::Error::new( | ||
| io::ErrorKind::InvalidData, | ||
| format!("package {package_name} appears more than once in the lockfile"), | ||
|
Comment on lines
+83
to
+86
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 1. Ambiguous package error untested The new duplicate-package rejection in select_locked_package has no automated test that constructs an ambiguous lockfile and asserts the CLI error behavior. This leaves a distinct business-logic conflict branch unverified. Agent Prompt
|
||
| )); | ||
| } | ||
| Ok(selected) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3. Inputs bypass size bounds
🐞 Bug☼ ReliabilityAgent Prompt
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools