Skip to content
Merged
70 changes: 70 additions & 0 deletions .github/workflows/cf12-impact-proof.yml
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
90 changes: 90 additions & 0 deletions crates/commandf-cli/src/impact.rs
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)?)?;
Comment on lines +18 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

3. Inputs bypass size bounds 🐞 Bug ☼ Reliability

impact::run uses unrestricted fs::read for user-supplied lockfiles and cache archives, allowing
oversized local inputs to allocate memory before parsing or bounded archive scanning can reject
them. The archive scanner's 512 MiB limit applies only to decompressed traversal after the complete
compressed archive is already resident in memory.
Agent Prompt
## Issue description
The new CLI reads arbitrary lock and cache files fully into memory before enforcing any input-size or decompression-work limits.

## Issue Context
Add explicit persisted-lock and compressed-archive byte limits at the read boundary, rejecting oversized files before allocation. Preserve the existing decompressed-byte, entry-count, and resource-size checks.

## Fix Focus Areas
- crates/commandf-cli/src/impact.rs[18-19]
- crates/commandf-cli/src/impact.rs[27-31]
- crates/commandf-pkg/src/cache.rs[67-85]
- crates/commandf-pkg/src/artifact_scan.rs[9-11]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Cache verification race 🐞 Bug ☼ Reliability

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
## Issue description
The impact CLI verifies a cache object and then rereads it outside `PackageCache`'s digest-checking boundary, allowing a check/use race and performing redundant I/O.

## Issue Context
`PackageCache::read_verified` returns the exact bytes whose digest it checked. Keep those bytes and pass them to the diff instead of calling `verify` followed by `fs::read`.

## Fix Focus Areas
- crates/commandf-cli/src/impact.rs[27-31]
- crates/commandf-cli/src/impact.rs[60-67]
- crates/commandf-pkg/src/cache.rs[63-85]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

1. Ambiguous package error untested 📘 Rule violation ▣ Testability

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
## Issue description
Add deterministic automated coverage for the `commandf impact` branch that rejects duplicate entries for the selected package.

## Issue Context
Construct a schema-v2 lockfile containing the requested package more than once, invoke the shipped CLI, and assert the nonzero exit status, empty stdout, and exact duplicate-package diagnostic.

## Fix Focus Areas
- crates/commandf-cli/src/impact.rs[83-87]
- crates/commandf-cli/tests/impact_behavior.rs[73-107]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

));
}
Ok(selected)
}
27 changes: 27 additions & 0 deletions crates/commandf-cli/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
mod impact;
mod oracle;

use std::ffi::OsStr;
Expand Down Expand Up @@ -69,6 +70,19 @@ enum Command {
#[arg(long, value_enum, default_value = "json")]
format: OutputFormat,
},
Impact {
package: String,
#[arg(long)]
before_lock: PathBuf,
#[arg(long)]
before_cache: PathBuf,
#[arg(long)]
after_lock: PathBuf,
#[arg(long)]
after_cache: PathBuf,
#[arg(long, value_enum, default_value = "json")]
format: OutputFormat,
},
Classify {
package: String,
#[arg(long)]
Expand Down Expand Up @@ -353,6 +367,19 @@ fn run(cli: Cli) -> Result<ExitCode, Box<dyn std::error::Error>> {
OutputFormat::Json => io::stdout().write_all(&report.to_json_bytes()?)?,
}
}
Command::Impact {
package,
before_lock,
before_cache,
after_lock,
after_cache,
format,
} => {
let bytes = impact::run(package, before_lock, before_cache, after_lock, after_cache)?;
match format {
OutputFormat::Json => io::stdout().write_all(&bytes)?,
}
}
Command::Classify {
package,
before_lock,
Expand Down
Loading
Loading