Skip to content

Commit 7d57588

Browse files
PhysShellclaude
andcommitted
feat(migrate): green — output-root safety + pinned v9 schema target
Close the three gaps the arbiter flagged on #162: 1. "Never in place" is now enforced. `preflight_output` runs before any input is read: it canonicalizes corpus, tabs, and the prospective output root (resolving symlink aliases), and refuses when the output already exists, equals an input root, or nests with either input root in either direction. Every refusal leaves both inputs byte-identical. 2. The migrator no longer writes the moving `SCHEMA_VERSION`. Output manifests are stamped with a pinned `TARGET_SCHEMA_VERSION = 9`, and an input manifest newer than the target is refused rather than downgraded — so a recompile after v10 cannot forge a v10 stamp. 3. The load-bearing I/O contract is now covered by committed end-to-end tests that drive `run()` over real temporary trees, not just the pure helpers. 23/23 tests green; clippy (all=deny) and rustfmt clean. `load_corpus` and `guard_schema` extracted to keep `run()` within the line budget. README updated to match the enforced contract. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012T7SRMiXmZe5v1imtrKMPi
1 parent 08f4875 commit 7d57588

2 files changed

Lines changed: 168 additions & 26 deletions

File tree

‎migrate/README.md‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,24 @@ follow-up work.
3131

3232
## Contract (enforced by the tests in `src/main.rs`)
3333

34+
The pure planning helpers (`resolve_source`, `build_plan`, `apply_plan`) and the
35+
whole-tool `run()` are both covered — the end-to-end tests build temporary
36+
`corpus/` + `tabs/` trees and drive `run()` against them.
37+
3438
- **Unambiguous join** — a source resolves to exactly one tab (by basename, then
3539
by extension-insensitive stem). Byte-identical duplicate tabs are **not**
3640
ambiguous; their digest is determined.
3741
- **Fail closed, no partial write** — if *any* chunk is unresolved (missing or
3842
ambiguous) or conflicts with a digest already recorded, the whole run refuses
39-
and writes nothing. Verified: a refused run creates no output directory.
43+
and writes nothing. An end-to-end test asserts a refused run creates no output.
44+
- **Never in place** — before any input is read, a preflight canonicalizes all
45+
three roots (resolving symlink aliases) and refuses when the output already
46+
exists, equals an input root, or nests with either input root in either
47+
direction. Every such refusal leaves the inputs byte-identical.
48+
- **Pinned target** — the migrator writes exactly schema **v9**
49+
(`TARGET_SCHEMA_VERSION`), never the floating `griff_core` `SCHEMA_VERSION`, so
50+
a recompile after v10 cannot silently forge a v10 stamp. Inputs newer than the
51+
target are refused, not downgraded.
4052
- **Idempotent** — re-running over already-backfilled records is a no-op.
4153
- **Consistent** — two chunks naming the same source receive the same digest.
4254
- **Verifiable** — the recorded digest equals what the loader recomputes

‎migrate/src/main.rs‎

Lines changed: 155 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,18 @@
3232
//! - **Consistent.** Two chunks naming the same source file receive the same
3333
//! digest.
3434
35-
use griff_core::corpus::{source_sha256, ChunkMeta, CorpusManifest, SCHEMA_VERSION};
35+
use griff_core::corpus::{source_sha256, ChunkMeta, CorpusManifest};
3636
use std::collections::BTreeMap;
3737
use std::path::{Path, PathBuf};
3838
use std::process::ExitCode;
3939

40+
/// The schema version this migrator produces. Pinned deliberately: it must not
41+
/// float with `griff_core::corpus::SCHEMA_VERSION`. A v9 migrator recompiled
42+
/// after v10 lands would otherwise stamp its output as v10 without populating or
43+
/// validating v10's contract — a time-delayed schema forgery. Inputs newer than
44+
/// this target are refused rather than downgraded.
45+
const TARGET_SCHEMA_VERSION: u32 = 9;
46+
4047
// ── tab index ─────────────────────────────────────────────────────────────────
4148

4249
/// A tab file discovered under the tabs root: its path relative to that root
@@ -246,6 +253,65 @@ fn render_errors(errors: &[MigrateError]) -> String {
246253

247254
// ── I/O orchestration ─────────────────────────────────────────────────────────
248255

256+
/// Enforce the "never in place" promise before any input is walked or any byte
257+
/// is written.
258+
///
259+
/// Canonicalizing resolves symlink aliases, so a textually distinct output that
260+
/// really lands inside an input tree is still caught. The output must not
261+
/// already exist (no silent reuse or overwrite), and neither input root may
262+
/// equal, contain, or be contained by the resolved output root.
263+
fn preflight_output(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String> {
264+
let corpus = corpus_dir
265+
.canonicalize()
266+
.map_err(|e| format!("corpus dir {}: {e}", corpus_dir.display()))?;
267+
let tabs = tabs_dir
268+
.canonicalize()
269+
.map_err(|e| format!("tabs dir {}: {e}", tabs_dir.display()))?;
270+
if out_dir.exists() {
271+
return Err(format!(
272+
"output {} already exists; refusing to reuse or overwrite it",
273+
out_dir.display()
274+
));
275+
}
276+
// The output does not exist yet, so resolve its parent and rejoin the final
277+
// component to obtain the real path it would occupy.
278+
let parent = match out_dir.parent() {
279+
Some(p) if !p.as_os_str().is_empty() => p,
280+
_ => Path::new("."),
281+
};
282+
let parent = parent
283+
.canonicalize()
284+
.map_err(|e| format!("output parent {}: {e}", parent.display()))?;
285+
let name = out_dir
286+
.file_name()
287+
.ok_or_else(|| format!("output {} has no final path component", out_dir.display()))?;
288+
let out = parent.join(name);
289+
290+
for (root, label) in [(&corpus, "corpus"), (&tabs, "tabs")] {
291+
if &out == root {
292+
return Err(format!(
293+
"output resolves to the {label} root {}; refusing in-place migration",
294+
root.display()
295+
));
296+
}
297+
if out.starts_with(root) {
298+
return Err(format!(
299+
"output {} resolves inside the {label} root {}; refusing",
300+
out.display(),
301+
root.display()
302+
));
303+
}
304+
if root.starts_with(&out) {
305+
return Err(format!(
306+
"{label} root {} resolves inside the output {}; refusing",
307+
root.display(),
308+
out.display()
309+
));
310+
}
311+
}
312+
Ok(())
313+
}
314+
249315
/// Recursively collect every file under `dir`, hard-failing on an unreadable
250316
/// directory rather than silently skipping it.
251317
fn walk(dir: &Path) -> Result<Vec<PathBuf>, String> {
@@ -282,15 +348,10 @@ fn write_text(out_dir: &Path, rel: &str, contents: &str) -> Result<(), String> {
282348
std::fs::write(&dest, contents).map_err(|e| format!("write {}: {e}", dest.display()))
283349
}
284350

285-
fn run(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String> {
286-
// 1. Index every tab by basename and stem, hashing its bytes once.
287-
let mut index = TabIndex::default();
288-
for path in walk(tabs_dir)? {
289-
let bytes = std::fs::read(&path).map_err(|e| format!("read {}: {e}", path.display()))?;
290-
index.insert(relpath(tabs_dir, &path), source_sha256(&bytes));
291-
}
292-
293-
// 2. Load the corpus: standalone chunk records and manifests.
351+
/// Load every `*.chunk.json` record and `manifest.json` under `corpus_dir`,
352+
/// each paired with its path relative to that root.
353+
type Corpus = (Vec<(String, ChunkMeta)>, Vec<(String, CorpusManifest)>);
354+
fn load_corpus(corpus_dir: &Path) -> Result<Corpus, String> {
294355
let mut chunk_files: Vec<(String, ChunkMeta)> = Vec::new();
295356
let mut manifests: Vec<(String, CorpusManifest)> = Vec::new();
296357
for path in walk(corpus_dir)? {
@@ -309,6 +370,46 @@ fn run(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String>
309370
manifests.push((rel, manifest));
310371
}
311372
}
373+
Ok((chunk_files, manifests))
374+
}
375+
376+
/// Refuse any manifest whose schema is newer than the pinned target — the
377+
/// migrator downgrades nothing and stamps nothing it did not populate.
378+
fn guard_schema(manifests: &[(String, CorpusManifest)]) -> Result<(), String> {
379+
let newer: Vec<String> = manifests
380+
.iter()
381+
.filter(|(_, m)| m.schema_version > TARGET_SCHEMA_VERSION)
382+
.map(|(rel, m)| {
383+
format!(
384+
" {rel}: input schema v{} is newer than the migration target v{TARGET_SCHEMA_VERSION}",
385+
m.schema_version
386+
)
387+
})
388+
.collect();
389+
if newer.is_empty() {
390+
return Ok(());
391+
}
392+
Err(format!(
393+
"refusing to migrate: {} manifest(s) newer than target v{TARGET_SCHEMA_VERSION}\n{}",
394+
newer.len(),
395+
newer.join("\n")
396+
))
397+
}
398+
399+
fn run(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String> {
400+
// 0. Refuse dangerous output roots before touching any input.
401+
preflight_output(corpus_dir, tabs_dir, out_dir)?;
402+
403+
// 1. Index every tab by basename and stem, hashing its bytes once.
404+
let mut index = TabIndex::default();
405+
for path in walk(tabs_dir)? {
406+
let bytes = std::fs::read(&path).map_err(|e| format!("read {}: {e}", path.display()))?;
407+
index.insert(relpath(tabs_dir, &path), source_sha256(&bytes));
408+
}
409+
410+
// 2. Load the corpus, then refuse any input newer than the pinned target.
411+
let (chunk_files, manifests) = load_corpus(corpus_dir)?;
412+
guard_schema(&manifests)?;
312413

313414
// 3. Validate everything before writing anything (fail closed).
314415
let mut errors: Vec<MigrateError> = Vec::new();
@@ -335,8 +436,8 @@ fn run(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String>
335436
return Err(render_errors(&errors));
336437
}
337438

338-
// 4. Apply and write. Chunk files keep their format; manifests also advance
339-
// `schema_version` to the current v9.
439+
// 5. Apply and write. Chunk files keep their format; manifests also advance
440+
// `schema_version` to the pinned v9 target.
340441
apply_plan(&mut chunk_metas, &chunk_plan);
341442
for ((rel, _), meta) in chunk_files.iter().zip(&chunk_metas) {
342443
let json =
@@ -346,7 +447,7 @@ fn run(corpus_dir: &Path, tabs_dir: &Path, out_dir: &Path) -> Result<(), String>
346447
for ((rel, manifest), plan) in manifests.iter().zip(&manifest_plans) {
347448
let mut migrated = manifest.clone();
348449
apply_plan(&mut migrated.chunks, plan);
349-
migrated.schema_version = SCHEMA_VERSION;
450+
migrated.schema_version = TARGET_SCHEMA_VERSION;
350451
let json =
351452
serde_json::to_string_pretty(&migrated).map_err(|e| format!("serialize {rel}: {e}"))?;
352453
write_text(out_dir, rel, &json)?;
@@ -652,11 +753,23 @@ mod tests {
652753

653754
/// Build a `corpus/` (one chunk file + one manifest) and a `tabs/` tree under
654755
/// `tree`, and return the corpus/tabs paths plus the tab's true digest.
655-
fn seed(tree: &TmpTree, schema_version: u32, filename: &str, tab_bytes: &str) -> (PathBuf, PathBuf, String) {
756+
fn seed(
757+
tree: &TmpTree,
758+
schema_version: u32,
759+
filename: &str,
760+
tab_bytes: &str,
761+
) -> (PathBuf, PathBuf, String) {
656762
tree.write("corpus/e2e_001.p0.chunk.json", &chunk_json(filename));
657-
tree.write("corpus/manifest.json", &manifest_json(schema_version, filename));
763+
tree.write(
764+
"corpus/manifest.json",
765+
&manifest_json(schema_version, filename),
766+
);
658767
tree.write(&format!("tabs/{filename}"), tab_bytes);
659-
(tree.path("corpus"), tree.path("tabs"), source_sha256(tab_bytes.as_bytes()))
768+
(
769+
tree.path("corpus"),
770+
tree.path("tabs"),
771+
source_sha256(tab_bytes.as_bytes()),
772+
)
660773
}
661774

662775
fn out_chunk_sha(out: &Path) -> Option<String> {
@@ -677,13 +790,24 @@ mod tests {
677790
&std::fs::read_to_string(out.join("e2e_001.p0.chunk.json")).unwrap(),
678791
)
679792
.unwrap();
680-
assert_eq!(chunk.source.track_index, None, "track_index must stay absent");
681-
let manifest: CorpusManifest = serde_json::from_str(
682-
&std::fs::read_to_string(out.join("manifest.json")).unwrap(),
683-
)
684-
.unwrap();
685-
assert_eq!(manifest.schema_version, 9, "manifest must become exactly v9");
686-
assert_eq!(manifest.chunks.first().and_then(|c| c.source.sha256.as_deref()), Some(sha.as_str()));
793+
assert_eq!(
794+
chunk.source.track_index, None,
795+
"track_index must stay absent"
796+
);
797+
let manifest: CorpusManifest =
798+
serde_json::from_str(&std::fs::read_to_string(out.join("manifest.json")).unwrap())
799+
.unwrap();
800+
assert_eq!(
801+
manifest.schema_version, 9,
802+
"manifest must become exactly v9"
803+
);
804+
assert_eq!(
805+
manifest
806+
.chunks
807+
.first()
808+
.and_then(|c| c.source.sha256.as_deref()),
809+
Some(sha.as_str())
810+
);
687811
}
688812

689813
#[test]
@@ -743,7 +867,10 @@ mod tests {
743867
std::fs::create_dir_all(&out).unwrap();
744868
std::fs::write(out.join("sentinel"), "keep").unwrap();
745869
run(&corpus, &tabs, &out).expect_err("existing output must refuse");
746-
assert!(out.join("sentinel").exists(), "existing output left untouched");
870+
assert!(
871+
out.join("sentinel").exists(),
872+
"existing output left untouched"
873+
);
747874
}
748875

749876
#[test]
@@ -757,7 +884,10 @@ mod tests {
757884
std::os::unix::fs::symlink(&corpus, &alias).unwrap();
758885
let out = alias.join("nested_out");
759886
run(&corpus, &tabs, &out).expect_err("symlink alias into corpus must refuse");
760-
assert!(!corpus.join("nested_out").exists(), "nothing written into corpus");
887+
assert!(
888+
!corpus.join("nested_out").exists(),
889+
"nothing written into corpus"
890+
);
761891
}
762892

763893
#[test]

0 commit comments

Comments
 (0)