fix(12): revise plans based on checker feedback
This commit is contained in:
@@ -36,6 +36,9 @@ must_haves:
|
||||
- path: "src/archive.rs"
|
||||
provides: "pack/unpack/inspect accept key parameter"
|
||||
contains: "key: &[u8; 32]"
|
||||
- path: "src/archive.rs"
|
||||
provides: "inspect accepts optional key for TOC decryption"
|
||||
contains: "key: Option<&[u8; 32]>"
|
||||
- path: "src/main.rs"
|
||||
provides: "Wiring: CLI -> key resolution -> archive functions"
|
||||
key_links:
|
||||
@@ -227,19 +230,18 @@ pub fn resolve_key(source: &KeySource) -> anyhow::Result<[u8; 32]> {
|
||||
}
|
||||
```
|
||||
|
||||
4. **src/archive.rs**: Refactor all three public functions to accept `key: &[u8; 32]` parameter:
|
||||
4. **src/archive.rs**: Refactor all three public functions to accept a `key` parameter:
|
||||
- `pub fn pack(files: &[PathBuf], output: &Path, no_compress: &[String], key: &[u8; 32])`
|
||||
- `pub fn unpack(archive: &Path, output_dir: &Path, key: &[u8; 32])`
|
||||
- `pub fn inspect(archive: &Path, key: Option<&[u8; 32]>)` -- key is optional for inspect
|
||||
- `pub fn inspect(archive: &Path, key: Option<&[u8; 32]>)` -- key is **optional** for inspect (KEY-07)
|
||||
- Remove `use crate::key::KEY;` import
|
||||
- Change `read_archive_metadata` to accept `key: &[u8; 32]` parameter
|
||||
- Change `read_archive_metadata` to accept `key: Option<&[u8; 32]>` parameter
|
||||
- Update `process_file` to accept `key: &[u8; 32]` parameter
|
||||
- Replace all `&KEY` references with the passed-in `key` parameter
|
||||
- For `inspect`: when key is `None`, try to read header+TOC but skip TOC decryption if encrypted (show "TOC encrypted, provide key to inspect entries"); when key is `Some(k)`, use it for full inspection.
|
||||
- For `inspect` when key is `None`: read and display header fields (version, flags, file_count, toc_offset, whether salt/KDF is present) WITHOUT attempting TOC decryption. If the TOC is encrypted (flags bit 1), print "TOC is encrypted, provide a key to see entry listing". If the TOC is NOT encrypted, parse and display entries normally.
|
||||
- For `inspect` when key is `Some(k)`: decrypt TOC and show full entry listing (file names, sizes, compression flags, etc.).
|
||||
|
||||
Actually, inspect DOES need the key for encrypted TOC decryption. So inspect should also require a key. But the phase goal says "All three methods produce a 32-byte AES-256 key passed through pack/unpack/inspect." So inspect also requires a key. Change inspect signature to `pub fn inspect(archive: &Path, key: &[u8; 32])`.
|
||||
|
||||
5. **src/main.rs**: Wire CLI args to key resolution and archive functions:
|
||||
5. **src/main.rs**: Wire CLI args to key resolution and archive functions. **CRITICAL**: `inspect` must work WITHOUT a key (KEY-07). Only `pack` and `unpack` require a key argument.
|
||||
|
||||
```rust
|
||||
use encrypted_archive::key::{KeySource, resolve_key};
|
||||
@@ -247,28 +249,37 @@ use encrypted_archive::key::{KeySource, resolve_key};
|
||||
fn main() -> anyhow::Result<()> {
|
||||
let cli = Cli::parse();
|
||||
|
||||
// Determine key source from CLI args
|
||||
// Determine key source from CLI args (may be None for inspect)
|
||||
let key_source = if let Some(hex) = &cli.key_args.key {
|
||||
KeySource::Hex(hex.clone())
|
||||
Some(KeySource::Hex(hex.clone()))
|
||||
} else if let Some(path) = &cli.key_args.key_file {
|
||||
KeySource::File(path.clone())
|
||||
Some(KeySource::File(path.clone()))
|
||||
} else if let Some(password_opt) = &cli.key_args.password {
|
||||
KeySource::Password(password_opt.clone())
|
||||
Some(KeySource::Password(password_opt.clone()))
|
||||
} else {
|
||||
anyhow::bail!("One of --key, --key-file, or --password is required")
|
||||
None
|
||||
};
|
||||
|
||||
let key = resolve_key(&key_source)?;
|
||||
|
||||
match cli.command {
|
||||
Commands::Pack { files, output, no_compress } => {
|
||||
let source = key_source
|
||||
.ok_or_else(|| anyhow::anyhow!("One of --key, --key-file, or --password is required for pack"))?;
|
||||
let key = resolve_key(&source)?;
|
||||
archive::pack(&files, &output, &no_compress, &key)?;
|
||||
}
|
||||
Commands::Unpack { archive: arch, output_dir } => {
|
||||
let source = key_source
|
||||
.ok_or_else(|| anyhow::anyhow!("One of --key, --key-file, or --password is required for unpack"))?;
|
||||
let key = resolve_key(&source)?;
|
||||
archive::unpack(&arch, &output_dir, &key)?;
|
||||
}
|
||||
Commands::Inspect { archive: arch } => {
|
||||
archive::inspect(&arch, &key)?;
|
||||
// Inspect works without a key (shows header metadata only).
|
||||
// With a key, it also decrypts and shows the TOC entry listing.
|
||||
let key = key_source
|
||||
.map(|s| resolve_key(&s))
|
||||
.transpose()?;
|
||||
archive::inspect(&arch, key.as_ref())?;
|
||||
}
|
||||
}
|
||||
|
||||
@@ -350,7 +361,9 @@ fn main() -> anyhow::Result<()> {
|
||||
- `test_key_file_roundtrip`: Create a 32-byte key file, pack with `--key-file`, unpack with `--key-file`, verify byte-identical.
|
||||
- `test_rejects_wrong_key`: Pack with one key, try unpack with different key, expect HMAC failure.
|
||||
- `test_rejects_bad_hex`: Run with `--key abcd` (too short), expect error.
|
||||
- `test_rejects_missing_key`: Run `pack file -o out` without any key arg, expect error.
|
||||
- `test_rejects_missing_key`: Run `pack file -o out` without any key arg, expect error about "required for pack".
|
||||
- `test_inspect_without_key`: Pack with --key, then run `inspect` WITHOUT any key arg. Should succeed and print header metadata (version, flags, file_count). Should NOT show decrypted TOC entries.
|
||||
- `test_inspect_with_key`: Pack with --key, then run `inspect --key <hex>`. Should succeed and print both header metadata AND full TOC entry listing.
|
||||
|
||||
5. Run full test suite: `cargo test` -- all tests must pass.
|
||||
</action>
|
||||
@@ -363,7 +376,9 @@ fn main() -> anyhow::Result<()> {
|
||||
- New test: key file round-trip works
|
||||
- New test: wrong key causes HMAC failure
|
||||
- New test: bad hex rejected with clear error
|
||||
- New test: missing key arg rejected with clear error
|
||||
- New test: missing key arg rejected with clear error for pack/unpack
|
||||
- New test: inspect without key shows header metadata only
|
||||
- New test: inspect with key shows full TOC entry listing
|
||||
- `cargo test` reports 0 failures
|
||||
</done>
|
||||
</task>
|
||||
@@ -374,14 +389,17 @@ fn main() -> anyhow::Result<()> {
|
||||
1. `cargo build` succeeds
|
||||
2. `cargo test` all pass (0 failures)
|
||||
3. Manual smoke test: `cargo run -- --key 7a35c1d94fe82b6a910df358bc74a61e428fd063e5179b2cfa8406cd3e79b550 pack README.md -o /tmp/test.aea && cargo run -- --key 7a35c1d94fe82b6a910df358bc74a61e428fd063e5179b2cfa8406cd3e79b550 unpack /tmp/test.aea -o /tmp/test_out`
|
||||
4. Inspect works: `cargo run -- --key 7a35c1d94fe82b6a910df358bc74a61e428fd063e5179b2cfa8406cd3e79b550 inspect /tmp/test.aea`
|
||||
5. Missing key rejected: `cargo run -- pack README.md -o /tmp/test.aea` should fail
|
||||
6. Bad hex rejected: `cargo run -- --key abcd pack README.md -o /tmp/test.aea` should fail
|
||||
4. Inspect with key: `cargo run -- --key 7a35c1d94fe82b6a910df358bc74a61e428fd063e5179b2cfa8406cd3e79b550 inspect /tmp/test.aea` shows full entry listing
|
||||
5. Inspect without key: `cargo run -- inspect /tmp/test.aea` shows header metadata only (no entry listing, prints "TOC is encrypted, provide a key to see entry listing")
|
||||
6. Missing key rejected for pack: `cargo run -- pack README.md -o /tmp/test.aea` should fail with "required for pack"
|
||||
7. Missing key rejected for unpack: `cargo run -- unpack /tmp/test.aea -o /tmp/out` should fail with "required for unpack"
|
||||
8. Bad hex rejected: `cargo run -- --key abcd pack README.md -o /tmp/test.aea` should fail
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
- Hardcoded KEY constant is no longer used in production code (only in test constants)
|
||||
- `--key <HEX>` and `--key-file <PATH>` work for pack/unpack/inspect
|
||||
- `--key <HEX>` and `--key-file <PATH>` work for pack/unpack and optionally for inspect
|
||||
- `inspect` works without any key argument (shows header metadata), and with a key (shows full TOC listing)
|
||||
- `--password` is accepted by CLI but returns "not yet implemented" error
|
||||
- All existing tests pass with explicit key arguments
|
||||
- New tests verify key-file, wrong-key rejection, bad-hex rejection, missing-key rejection
|
||||
|
||||
@@ -10,6 +10,7 @@ files_modified:
|
||||
- src/key.rs
|
||||
- src/format.rs
|
||||
- src/archive.rs
|
||||
- src/main.rs
|
||||
- tests/round_trip.rs
|
||||
autonomous: true
|
||||
requirements:
|
||||
@@ -89,7 +90,7 @@ From src/archive.rs (after Plan 01):
|
||||
```rust
|
||||
pub fn pack(files: &[PathBuf], output: &Path, no_compress: &[String], key: &[u8; 32]) -> anyhow::Result<()>
|
||||
pub fn unpack(archive: &Path, output_dir: &Path, key: &[u8; 32]) -> anyhow::Result<()>
|
||||
pub fn inspect(archive: &Path, key: &[u8; 32]) -> anyhow::Result<()>
|
||||
pub fn inspect(archive: &Path, key: Option<&[u8; 32]>) -> anyhow::Result<()>
|
||||
```
|
||||
|
||||
From src/format.rs (current):
|
||||
|
||||
Reference in New Issue
Block a user