[GH-ISSUE #2620] fix(artifacts): directory inventory accepts descendants of a replaced root #936

New issue

Closed

opened 2026-10-15 18:29:42 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-15 18:29:42 +00:00

Owner

Originally created by @coygeek on GitHub (Oct 13, 2026).
Original GitHub issue: https://github.com/morluto/rea/issues/2620

Summary

Replacing a selected directory with another real directory at the same path during inventory lets traversal continue into the replacement. The resulting inventory reports complete coverage even though its first entry came from the original tree and its descendant file came from the new tree.

Steps to reproduce

Save the following test as src/artifacts/replaced-root.probe.test.ts and run npm run test:focused -- src/artifacts/replaced-root.probe.test.ts. The first case pauses the actual reader after its first entry. The second schedules the same update through a reader subclass while the production inventory pipeline consumes real entries. All filesystem changes remain inside the disposable fixture.

import { mkdir, rename, writeFile } from 'node:fs/promises';
import { join } from 'node:path';
import { expect, it } from 'vitest';
import { DirectoryArtifactReader } from './DirectoryArtifactReader.js';
import { createTestTempDirectory } from '../../tests/fixtures/temporaryDirectory.js';
import { inventoryArtifact, scanCanonicalArtifactInventory } from '../../tests/fixtures/artifactInventory.js';
const fixture=async()=>{
 const parent=await createTestTempDirectory('directory-replace-');
 const root=join(parent,'root');
 await mkdir(join(root,'sub'),{recursive:true});
 await writeFile(join(root,'sub','old.txt'),'old');
 return {root,replace:async()=>{
   await rename(root,join(parent,'original'));
   await mkdir(join(root,'sub'),{recursive:true});
   await writeFile(join(root,'sub','new.txt'),'new');
 }};
};
it('same iterator continues into a replaced real root',async()=>{
 const {root,replace}=await fixture();
 const reader=new DirectoryArtifactReader(root);
 const iterator=reader.entries()[Symbol.asyncIterator]();
 try {
   expect((await iterator.next()).value?.path).toBe('sub');
   await replace();
   const rest=[];
   for(let next=await iterator.next();!next.done;next=await iterator.next()) rest.push(next.value.path);
   expect(rest).toEqual(['sub/new.txt']);
 } finally {await iterator.return?.();await reader.close();}
});
it('inventory reports replacement descendants as complete',async()=>{
 const {root,replace}=await fixture();
 class UpdatingReader extends DirectoryArtifactReader {
   override async *entries(signal?:AbortSignal){
     let replaced=false;
     for await(const entry of super.entries(signal)) {
       yield entry;
       if(!replaced){replaced=true;await replace();}
     }
   }
 }
 const inventory=await scanCanonicalArtifactInventory(root,{},()=>new UpdatingReader(root));
 expect(inventory.coverage_status).toBe('complete');
 expect(inventory.occurrences.map(o=>o.logical_path)).toContain('sub/new.txt');
 expect(inventory.occurrences.map(o=>o.logical_path)).not.toContain('sub/old.txt');
 const stable=await inventoryArtifact(root);
 expect(stable.coverage_status).toBe('complete');
 expect(stable.occurrences.map(o=>o.logical_path)).toEqual(inventory.occurrences.map(o=>o.logical_path));
 const preserved=await inventoryArtifact(join(root,'..','original'));
 expect(preserved.coverage_status).toBe('complete');
 expect(preserved.occurrences.map(o=>o.logical_path)).toContain('sub/old.txt');
 expect(preserved.occurrences.map(o=>o.logical_path)).not.toContain('sub/new.txt');
});

Expected behavior

After the selected root changes directory identity, the operation should surface the change or remain bound to the originally selected tree. It must not silently accept replacement descendants as a complete observation of the original tree. An unchanged root should still enumerate and inventory normally.

Actual behavior

Both probe cases completed without an identity failure. The direct iterator first returned sub from the original directory, then returned sub/new.txt from the replacement. Production inventory included that replacement file, omitted sub/old.txt, and reported coverage_status: complete. A later inventory of the settled replacement also completed normally. A separate inventory at the retained original directory’s known new path completed with sub/old.txt and without sub/new.txt.

Evidence

Expected source: AGENTS.md, Architecture and Cleanup, requires preservation of resource identity and source evidence; src/artifacts/DirectoryArtifactReader.ts:45-61 checks directory containment around traversal and src/artifacts/DirectoryArtifactReader.ts:170-182 rejects changed file identity.
Failure source: src/artifacts/DirectoryArtifactReader.ts:64-74 caches only a canonical root path and src/artifacts/DirectoryArtifactReader.ts:413-431 checks containment/type without captured directory identity. src/artifacts/inventory/classify.ts:81-82 gives directory roots a null digest, so src/artifacts/inventory/scanCanonical.ts:329-336 skips its final root check.
Evidence provenance: observed
Local verification: reproduced
Reproduction completeness: complete

Restoration check

Repeat both update cases. Neither may produce replacement descendants as a successful complete inventory of the originally selected tree. A changed-root error or traversal that stays bound to the original tree restores this contract. Preserve normal unchanged-root traversal and existing file-replacement rejection.

Runtime or environment

Source revision 8b181716aca2cb467be499cba226abfb5ae19141, Node v26.10.0, npm 11.19.1, Vitest 5.0.3, and the installed lockfile dependencies.

Triage assessment

Impact level: P2
Assessment status: supported
Impact basis: A concurrent directory replacement silently changes the tree contributing to a complete inventory and undermines its identity evidence. The probe establishes this bounded update sequence, not the frequency in real workloads.
Workaround status: available
Workaround basis: For this rename-and-recreation trigger, the original tree remains at a known path. The executed recovery control inventories that retained original and returns its original descendant rather than the replacement file. Users can restart inventory at the retained original’s new path. This does not repair the earlier mixed result or establish recovery when an external updater deletes or hides the original.

Originally created by @coygeek on GitHub (Oct 13, 2026). Original GitHub issue: https://github.com/morluto/rea/issues/2620 ## Summary Replacing a selected directory with another real directory at the same path during inventory lets traversal continue into the replacement. The resulting inventory reports complete coverage even though its first entry came from the original tree and its descendant file came from the new tree. ## Steps to reproduce Save the following test as `src/artifacts/replaced-root.probe.test.ts` and run `npm run test:focused -- src/artifacts/replaced-root.probe.test.ts`. The first case pauses the actual reader after its first entry. The second schedules the same update through a reader subclass while the production inventory pipeline consumes real entries. All filesystem changes remain inside the disposable fixture. ```ts import { mkdir, rename, writeFile } from 'node:fs/promises'; import { join } from 'node:path'; import { expect, it } from 'vitest'; import { DirectoryArtifactReader } from './DirectoryArtifactReader.js'; import { createTestTempDirectory } from '../../tests/fixtures/temporaryDirectory.js'; import { inventoryArtifact, scanCanonicalArtifactInventory } from '../../tests/fixtures/artifactInventory.js'; const fixture=async()=>{ const parent=await createTestTempDirectory('directory-replace-'); const root=join(parent,'root'); await mkdir(join(root,'sub'),{recursive:true}); await writeFile(join(root,'sub','old.txt'),'old'); return {root,replace:async()=>{ await rename(root,join(parent,'original')); await mkdir(join(root,'sub'),{recursive:true}); await writeFile(join(root,'sub','new.txt'),'new'); }}; }; it('same iterator continues into a replaced real root',async()=>{ const {root,replace}=await fixture(); const reader=new DirectoryArtifactReader(root); const iterator=reader.entries()[Symbol.asyncIterator](); try { expect((await iterator.next()).value?.path).toBe('sub'); await replace(); const rest=[]; for(let next=await iterator.next();!next.done;next=await iterator.next()) rest.push(next.value.path); expect(rest).toEqual(['sub/new.txt']); } finally {await iterator.return?.();await reader.close();} }); it('inventory reports replacement descendants as complete',async()=>{ const {root,replace}=await fixture(); class UpdatingReader extends DirectoryArtifactReader { override async *entries(signal?:AbortSignal){ let replaced=false; for await(const entry of super.entries(signal)) { yield entry; if(!replaced){replaced=true;await replace();} } } } const inventory=await scanCanonicalArtifactInventory(root,{},()=>new UpdatingReader(root)); expect(inventory.coverage_status).toBe('complete'); expect(inventory.occurrences.map(o=>o.logical_path)).toContain('sub/new.txt'); expect(inventory.occurrences.map(o=>o.logical_path)).not.toContain('sub/old.txt'); const stable=await inventoryArtifact(root); expect(stable.coverage_status).toBe('complete'); expect(stable.occurrences.map(o=>o.logical_path)).toEqual(inventory.occurrences.map(o=>o.logical_path)); const preserved=await inventoryArtifact(join(root,'..','original')); expect(preserved.coverage_status).toBe('complete'); expect(preserved.occurrences.map(o=>o.logical_path)).toContain('sub/old.txt'); expect(preserved.occurrences.map(o=>o.logical_path)).not.toContain('sub/new.txt'); }); ``` ## Expected behavior After the selected root changes directory identity, the operation should surface the change or remain bound to the originally selected tree. It must not silently accept replacement descendants as a complete observation of the original tree. An unchanged root should still enumerate and inventory normally. ## Actual behavior Both probe cases completed without an identity failure. The direct iterator first returned `sub` from the original directory, then returned `sub/new.txt` from the replacement. Production inventory included that replacement file, omitted `sub/old.txt`, and reported `coverage_status: complete`. A later inventory of the settled replacement also completed normally. A separate inventory at the retained original directory’s known new path completed with `sub/old.txt` and without `sub/new.txt`. ## Evidence - Expected source: `AGENTS.md`, Architecture and Cleanup, requires preservation of resource identity and source evidence; `src/artifacts/DirectoryArtifactReader.ts:45-61` checks directory containment around traversal and `src/artifacts/DirectoryArtifactReader.ts:170-182` rejects changed file identity. - Failure source: `src/artifacts/DirectoryArtifactReader.ts:64-74` caches only a canonical root path and `src/artifacts/DirectoryArtifactReader.ts:413-431` checks containment/type without captured directory identity. `src/artifacts/inventory/classify.ts:81-82` gives directory roots a null digest, so `src/artifacts/inventory/scanCanonical.ts:329-336` skips its final root check. - Evidence provenance: observed - Local verification: reproduced - Reproduction completeness: complete ## Restoration check Repeat both update cases. Neither may produce replacement descendants as a successful complete inventory of the originally selected tree. A changed-root error or traversal that stays bound to the original tree restores this contract. Preserve normal unchanged-root traversal and existing file-replacement rejection. ## Runtime or environment Source revision `8b181716aca2cb467be499cba226abfb5ae19141`, Node v26.10.0, npm 11.19.1, Vitest 5.0.3, and the installed lockfile dependencies. ## Triage assessment - Impact level: P2 - Assessment status: supported - Impact basis: A concurrent directory replacement silently changes the tree contributing to a complete inventory and undermines its identity evidence. The probe establishes this bounded update sequence, not the frequency in real workloads. - Workaround status: available - Workaround basis: For this rename-and-recreation trigger, the original tree remains at a known path. The executed recovery control inventories that retained original and returns its original descendant rather than the replacement file. Users can restart inventory at the retained original’s new path. This does not repair the earlier mixed result or establish recovery when an external updater deletes or hides the original.

bsakaguchi

2026-10-15 18:29:42 +00:00

closed this issue
added the
area: evidence

bug
labels

bsakaguchi commented

2026-10-15 18:29:43 +00:00

Author

Owner

@morluto commented on GitHub (Oct 14, 2026):

Addressed on main in ec271121e.

Directory readers bind traversal and child reads to the selected root device/inode, rejecting replacement trees.

Validation: Six directory reader ownership regressions passed.

Verification scope: This does not introduce target mutation authority.

@morluto commented on GitHub (Oct 14, 2026): Addressed on main in [ec271121e](https://github.com/morluto/rea/commit/ec271121e). Directory readers bind traversal and child reads to the selected root device/inode, rejecting replacement trees. Validation: Six directory reader ownership regressions passed. Verification scope: This does not introduce target mutation authority.

bsakaguchi referenced this issue

2026-10-17 21:07:01 +00:00

[PR #936] [CLOSED] feat(artifacts): inventory flat installer packages #2513

bsakaguchi referenced this issue

2026-10-17 21:08:25 +00:00

[PR #1082] [MERGED] fix(artifacts): keep cpio checksum evidence and bound xar TOC elements #2630

bsakaguchi referenced this issue

2026-10-17 21:08:26 +00:00

[PR #1086] [MERGED] fix(build): make catalogs and packaged skills generated outputs #2634