From b99bd38eb48cce50fed9c073af28a218a3354da0 Mon Sep 17 00:00:00 2001 From: Eric Rakestraw Date: Mon, 10 Aug 2026 02:45:51 +0000 Subject: [PATCH] Harden bundle promotion against symlink replacement --- docs/roadmap/implementation.md | 505 ++++++++++++++--------------- internal/fileops/directory.go | 162 +++++++-- internal/fileops/directory_test.go | 152 +++++++++ 3 files changed, 535 insertions(+), 284 deletions(-) diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 1f8f0f0..1228e45 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -2,8 +2,9 @@ ## Status And Audience -Complete. Stages 1 through 9 delivered the feature, and Stages 10 through 14 -closed the gaps found during the post-implementation architecture review. +Remediation required. Stages 1 through 14 are complete. Stages 15 through 18 +address the remaining security, lifecycle-state, and native-platform validation +gaps identified by the second post-implementation review. This plan is written for a GPT-5.6 Terra coding agent. Implement incomplete stages in strict numerical order. Do not skip ahead, combine stages merely to @@ -32,6 +33,10 @@ implementation sequence. | Stage 12 | Complete | | Stage 13 | Complete | | Stage 14 | Complete | +| Stage 15 | Complete | +| Stage 16 | Pending | +| Stage 17 | Pending | +| Stage 18 | Pending | After completing and validating a stage, update only that stage's row to `Complete` and record any material deviation in the relevant stage section. @@ -39,13 +44,12 @@ Do not mark a stage complete while required tests or exit criteria remain. ## Working Rules -Before Stage 10, read: +Before Stage 15, read: - `docs/development.md` and its task-specific references; -- `docs/roadmap/notarius-extract-stage.md` completely; -- `../notarius/docs/consumers/dnd-pipeline.md` and its linked subprocess, - receipt, JSON-output, and lane-contract documentation; and -- the focused Narratio documents and tests named by the current stage. +- `docs/roadmap/notarius-extract-stage.md` completely; and +- the focused Narratio documents, production code, and tests named by the + current stage. For every incomplete stage: @@ -57,304 +61,289 @@ For every incomplete stage: 4. Keep tests offline and independent of real Notarius, PromptKit, LLM providers, credentials, network services, and mutable external state. 5. Run the focused package tests listed for the stage, then `go test ./...`. -6. Run `go vet ./...` and `go build ./cmd/narratio` whenever shared contracts, - composition, CLI behavior, portability, or documentation examples change. +6. Run `go vet ./...` and build `cmd/narratio` whenever shared contracts, + composition, portability, or documentation examples change. Direct build + output to a temporary directory so validation does not leave an ignored + binary in the repository root. 7. Do not broaden the feature beyond the target architecture while fixing a - remediation. Prefer a general runner or filesystem invariant when the defect - is general, but avoid unrelated cleanup. + remediation. Prefer a general manifest or filesystem invariant when the + defect is general, but avoid unrelated cleanup. -## Completed Stages 1-9 Summary +## Completed Stages 1-14 Summary 1. **Stage 1 — Shared contracts:** Added explicit stage self-skip semantics, - artifact source identity, contract metadata, external provenance, and - backward-compatible manifest serialization. + artifact identities, contract metadata, provenance, and compatible manifest + serialization. 2. **Stage 2 — Configuration and source policy:** Added strict optional - Notarius configuration, deterministic defaults/path resolution, output-lane - contracts, and the `narratio.extraction.` source family. + Notarius configuration, deterministic path resolution, lane contracts, and + the `narratio.extraction.` source family. 3. **Stage 3 — Immutable promotion:** Added regular-file-only bundle copying, - symlink and special-file rejection, bounded permissions, syncing, cleanup, - and atomic no-replace installation on Linux. -4. **Stage 4 — Notarius adapter:** Added the subprocess boundary, exact CLI - invocation, separate receipt/diagnostic streams, bounded tolerant decoding, - path confinement, and generic receipt/index discovery. -5. **Stage 5 — Extract execution:** Added final-trimmed transcript consumption, - configured required-lane validation, immutable bundle promotion, checksums, - manifest-ready outputs, provenance, diagnostics, and invocation - fingerprinting. + static symlink and special-file rejection, syncing, cleanup, and atomic + no-replace installation on Linux. +4. **Stage 4 — Notarius adapter:** Added exact subprocess invocation, separate + receipt and diagnostic streams, bounded tolerant decoding, confinement, and + receipt/index discovery. +5. **Stage 5 — Extract execution:** Added transcript consumption, required-lane + validation, immutable promotion, checksums, outputs, provenance, + diagnostics, and invocation fingerprinting. 6. **Stage 6 — Lifecycle and resume:** Registered `extract` between `trim` and - `render`, added full and single-stage CLI support, resume validation, force - ordering, and old-manifest compatibility. + `render`, added CLI selection, resume validation, force behavior, and + old-manifest compatibility. 7. **Stage 7 — Analyze integration:** Added shared manifest-backed extraction catalog hydration and explicit required/optional Scriptorium input - resolution without directory scanning or implicit inputs. + resolution. 8. **Stage 8 — Publish and inspection integration:** Added explicit extraction publish rules, metadata round-tripping, restore compatibility, and operator availability reporting. -9. **Stage 9 — Documentation and examples:** Added maintained ten-lane and +9. **Stage 9 — Documentation and examples:** Added maintained complete and subset examples, integration and maintainer documentation, operational guidance, and repository-wide validation. +10. **Stage 10 — Replacement invalidation:** Generalized runner semantics so + forced, failed, skipped, changed, and non-resumable upstream executions + invalidate succeeded downstream results without causing perpetual reruns + for identical self-skips. +11. **Stage 11 — Explicit-only publication:** Excluded the run-local Notarius + bundle from run-record uploads while retaining diagnostics and explicit + configured-lane publication. +12. **Stage 12 — Canonical management files:** Required the production receipt + and index to use the canonical index, manifest, rejection, and warning + filenames assumed by extraction, resume, and catalog code. +13. **Stage 13 — Platform promotion contract:** Added atomic no-replace + implementations for Linux, macOS, and Windows, explicit early failure on + unsupported systems, and platform-specific directory syncing. +14. **Stage 14 — Documentation reconciliation:** Aligned current-behavior, + operations, troubleshooting, integration, internal, and roadmap + documentation with the implemented lifecycle and publication semantics. -## Stage 10: Correct Downstream Invalidation For Every Replaced Stage Result +## Stage 15: Make Bundle Promotion Race-Safe Against Symlink Replacement -Fix runner semantics so a newly produced extraction result cannot coexist with -downstream stages that are still considered current from an older result. Make -the correction at the general stage-runner boundary rather than adding an -`extract`-specific exception. +Close the gap between the documented no-symlink promotion invariant and the +current path-based recursive copy. A source directory that is replaced after +inspection must never cause Narratio to traverse a symlink or copy data from +outside the originally opened source tree. Implementation: -1. In `internal/app`, capture the prior session-stage outcome before changing a - selected stage to running. Preserve enough prior state to distinguish a - reused result, a repeated identical self-skip, and a replaced result. -2. Define replacement and invalidation behavior as follows: - - an already-succeeded stage skipped without execution does not invalidate - anything; - - a resume-validation error still stops without mutating the succeeded stage - or downstream stages; - - a non-resumable succeeded result is marked stale and invalidates succeeded - downstream stages before rerun, preserving the current behavior; - - any forced execution invalidates succeeded downstream stages even if the - execution later self-skips or fails; - - a non-forced execution that changes a skipped, failed, stale, pending, or - absent stage to succeeded invalidates succeeded downstream stages; - - an executed failure invalidates succeeded downstream stages before the - failed manifest state is committed; and - - a self-skip invalidates succeeded downstream stages only when it replaces - a different effective outcome. Repeating the same stable skip reason with - no outputs must not make every disabled full-pipeline invocation rerun all - downstream stages. -3. Persist invalidation with the same session-manifest transition that makes the - upstream replacement observable. Do not leave a committed failed or skipped - upstream result with succeeded downstream records that depend on its former - output. -4. Continue to use canonical stage order for invalidation. For `extract`, the - downstream set remains `render`, `analyze`, `publish`, and `notify`. -5. Recheck each precomputed stage decision immediately before acting on it. - When an earlier stage invalidates a later succeeded stage during the same - full run, the later stage must execute instead of honoring its original skip - decision. -6. Use stable bounded stale reasons that distinguish forced replacement, - changed upstream result, upstream failure, and upstream self-skip where that - distinction is operationally useful. -7. Remove or consolidate the old success-only force invalidation path after the - new invariant owns all replacement cases. Do not invalidate stages earlier - than the replaced stage. +1. Refactor `internal/fileops` source traversal to anchor the copy to an opened + source root for the entire operation. Use Go's `os.Root` APIs, available at + the module's declared Go version, instead of recursively reopening source + directories through unconstrained absolute paths. +2. Reject a source root that is itself a symlink. After opening the root, + compare the opened root identity with the identity inspected before opening; + fail if the source changed during that transition. +3. For every source entry: + - inspect it relative to the already opened parent root; + - reject symlinks, link-like Windows reparse points exposed as links, and + every non-regular, non-directory file type; + - open regular files relative to that root, compare the opened file identity + with the inspected identity, and copy only from the verified handle; and + - open child directories relative to the parent root, verify the opened + directory identity against the inspected identity, and recurse through the + opened child root rather than its pathname. +4. Treat any identity mismatch, disappearing entry, replacement, or unsupported + file type as a clean promotion failure. Preserve the source, remove the + temporary destination tree, and never install a partial destination. +5. Keep the destination-side behavior unchanged: deterministic traversal, + normalized permissions, file and directory syncing, sibling temporary tree, + and atomic no-replace installation. +6. Keep support bounded to Linux, macOS, and Windows. Do not weaken the strict + no-symlink rule to permit links that happen to remain inside the source root. +7. Keep test-only orchestration narrow. If deterministic replacement testing + requires an internal seam between inspection and opening, inject only that + boundary; do not expose it publicly or turn file copying into a generic + filesystem framework. Tests: -- extend `internal/app/extract_lifecycle_test.go` with a full-plan or focused - multi-stage fixture in which disabled `extract` and downstream `analyze` first - complete, Notarius is enabled, and the next ordinary run executes both - `extract` and the previously succeeded downstream stages; -- a failed extract followed by a successful ordinary retry invalidates and - reruns previously succeeded downstream stages; -- `run-stage extract --force` followed by `notarius_disabled` leaves downstream - succeeded stages stale; -- forced extraction failure leaves downstream succeeded stages stale; -- repeated identical disabled self-skip does not repeatedly stale downstream - stages; -- successful resume reuse does not invalidate downstream stages; -- resume-validation error preserves both the prior extract success and - downstream state; and -- forcing `render` still does not stale `extract`. +- retain the existing nested-copy, source-preservation, static symlink, + special-file, cleanup, existing-destination, and install-collision tests; +- deterministically replace an inspected child directory with a symlink before + it is opened, and prove promotion fails without copying an outside sentinel; +- cover source-root replacement between initial inspection and root opening; +- cover regular-file and directory identity mismatches without timing-based + sleeps or probabilistic race loops; +- prove every failure leaves the source and any concurrent destination intact + and removes the temporary sibling; and +- run the focused tests with the race detector. Exit criteria: -- Enabling, retrying, forcing, failing, or disabling extraction cannot leave an - incompatible succeeded analyze/publish result current. -- Repeated unchanged disabled extraction remains inexpensive and stable. -- `go test ./internal/app ./internal/stage ./internal/manifest`, - `go test ./...`, `go vet ./...`, and `go build ./cmd/narratio` pass. +- Recursive promotion cannot escape or switch away from the originally opened + source tree through a symlink replacement race. +- Static and concurrent symlink replacement are both protected by durable, + deterministic tests. +- `go test -count=1 ./internal/fileops`, + `go test -race -count=1 ./internal/fileops`, `go test ./...`, and + `go vet ./...` pass on the development platform. -## Stage 11: Exclude Run-Local Notarius Bundles From Implicit Publication +## Stage 16: Clear Superseded Session-Stage Result Payloads -Enforce the existing explicit-only publication boundary while preserving -ordinary run-record diagnostics. +Make the session manifest describe only the current stage outcome. Historical +successful result details belong to their immutable run manifests and must not +remain attached to a current stage record that is running, failed, skipped, or +newly succeeded without corresponding details. Implementation: -1. Update publish run-file collection so the canonical run-local - `extract/notarius-output/` subtree is never included in the run-record upload. - Match the exact slash-normalized relative subtree; do not use a broad - substring rule that could suppress unrelated files. -2. Keep the Notarius receipt and stderr files eligible for the existing run - archive. They are diagnostics, not the published bundle. Preserve current - audio exclusion and all unrelated run-file behavior. -3. Do not scan or upload the durable `artifacts/notarius//` directory. - A configured lane may still be uploaded only through an explicit - `pipeline.publish.outputs` rule resolved through the artifact catalog. -4. Preserve locks, required/optional output handling, upload ordering, current - manifest publication, and the final current-run pointer commit. -5. Update publish metadata so excluded staging-bundle files are not counted in - `run_files_uploaded` or listed in `run_uploaded_paths`. +1. Centralize clearing of result-bearing `manifest.StageRecord` fields: + `Outputs`, `Logs`, `GeneratedConfigs`, and `Metadata`. +2. Clear those fields when `MarkStageRunning` begins a replacement attempt. + The runner already captures the prior outcome before this transition; do not + change self-skip comparison or downstream invalidation decisions. +3. Also make `MarkStageFailed` and `MarkStageSkipped` enforce the empty-result + invariant themselves so callers cannot construct a failed or skipped record + with inherited successful output details. Avoid runner-only cleanup that + leaves direct manifest transitions inconsistent. +4. Do not clear result details merely when `MarkStageStale` is called. Resume + validation must still be able to inspect the prior result before deciding to + rerun it, and stale records remain useful diagnostic state until execution + actually begins. +5. Ensure `MarkStageSucceeded` followed by result application cannot inherit + old logs, generated configurations, or metadata when the new successful + result omits those fields. Clearing at the running transition should own this + invariant; do not add scattered empty-map special cases. +6. Preserve immutable prior run manifests and durable Notarius bundles. This + stage changes the current session-stage record, not historical files or + retention policy. +7. Consolidate the existing runner self-skip cleanup helper if it becomes + redundant after manifest transitions own the invariant. Tests: -- materialize a realistic current-run `extract/notarius-output//` - tree containing index, management, lane, pipeline-wide, and unknown regular - files, then prove none are uploaded under the run prefix; -- prove receipt and stderr diagnostics in the same extract directory remain - ordinary run uploads; -- prove no Notarius bundle member is uploaded when there is no explicit - extraction publish rule; -- prove one explicit extraction rule uploads only its durable selected lane to - the configured session destination; and -- retain assertions for commit-marker order, locks, previous-cache publication, - and unrelated run files. +- add manifest transition tests proving running, failed, and skipped records + cannot retain prior outputs, logs, generated configurations, or metadata; +- prove `MarkStageStale` retains the prior details required for resume + validation; +- change the forced-extraction-failure lifecycle fixture to first complete a + successful extraction, then force a failing replacement, and assert the + current extract record is failed with no inherited result payload; +- retain the assertion that succeeded downstream stages become stale; +- prove a successful replacement whose new result omits optional details does + not inherit details from its predecessor; and +- retain resume-error, repeated-self-skip, and ordinary-retry coverage. Exit criteria: -- No run-local or durable Notarius bundle is published wholesale. -- Explicit configured lane publication remains functional and auditable. -- `go test ./internal/stage ./internal/app ./internal/artifacts` and - `go test ./...` pass. +- A current running, failed, or skipped session-stage record never advertises + result payload belonging to an earlier success. +- Historical run manifests and immutable extraction bundles remain available + for audit and recovery. +- `go test -count=1 ./internal/manifest ./internal/app ./internal/stage`, + focused lifecycle race tests, `go test ./...`, and `go vet ./...` pass. -## Stage 12: Enforce Canonical Notarius Management-File Semantics +## Stage 17: Establish Native Promotion Validation On Every Supported Platform -Make the adapter's accepted production contract agree with extract, resume, and -catalog invariants. +Turn the Linux, macOS, and Windows promotion claim into a continuously verified +contract. Cross-compilation remains useful but cannot substitute for executing +filesystem operations on each native platform. Implementation: -1. Re-read Notarius's current `run-result.md` and `json-output.md` before editing - and keep tolerant decoding of unknown fields within the supported schema. -2. For `notarius.run-result.v1` using the production JSON output, require the - receipt's logical `index_file` to be exactly `index.json`. A different safe - relative path is semantically incompatible and must fail adapter discovery, - not produce a successful result that later becomes non-resumable. -3. Require the production index management fields to be exactly: - - `manifest_file: manifest.json`; - - `rejected_file: rejected.json`; and - - `warnings_file: warnings.json`. -4. Retain all existing confinement, regular-file, symlink, size-limit, and - tolerant unknown-field checks after the exact semantic checks. Do not tighten - optional lane descriptor fields beyond Notarius's published contract. -5. Keep the canonical index invariant shared by stage output, resume validation, - and catalog hydration. Avoid introducing a second configurable or inferred - management-path model. -6. Return errors that identify the incompatible field and observed value without - including transcript-derived payload content. - -Tests: - -- reject alternate but confined values such as `nested/index.json`, - `./index.json`, and renamed management files; -- continue rejecting absolute paths, traversal, symlinks, and missing files; -- continue accepting canonical documents with unknown optional fields; -- prove a successful adapter result promotes to canonical `index.json`; and -- prove a successful extract record is immediately accepted by resume validation - and catalog hydration without normalization or path rewriting. - -Exit criteria: - -- Every adapter success satisfies the canonical management-file assumptions of - extraction, resume, and catalog code. -- `go test ./internal/adapters/notarius ./internal/stage ./internal/artifacts` - and `go test ./...` pass. - -## Stage 13: Complete And Document The Atomic-Promotion Platform Contract - -Replace the accidental Linux-only behavior with an explicit, tested support -boundary. Support Linux, macOS, and Windows; fail early and clearly on other -platforms until they gain an atomic no-replace primitive. - -Implementation: - -1. Keep Linux installation based on `renameat2(RENAME_NOREPLACE)`. -2. Add a Darwin implementation using `renamex_np(RENAME_EXCL)` from - `golang.org/x/sys/unix`. -3. Add a Windows implementation using the no-replace form of - `golang.org/x/sys/windows.MoveFileEx`. The sibling temporary directory keeps - the move on one volume; do not set the replace-existing flag. -4. Split directory-sync behavior by platform where necessary. Unsupported - directory syncing may be treated as best effort only for documented - platform-specific unsupported-operation errors; do not suppress ordinary I/O - or permission failures. -5. Add a small build-specific capability check so an unsupported platform fails - before copying the bundle into a temporary tree. Retain safe cleanup if any - later platform operation fails. -6. Preserve the core invariant on every supported platform: a concurrent actor - that creates the destination wins or causes a clean error; Narratio never - replaces that destination. -7. Document Linux, macOS, and Windows as the supported atomic-promotion - platforms and the explicit extraction limitation on other operating systems. - Do not imply that all of Narratio has a broader support guarantee than its - existing documentation establishes. - -Tests and validation: - -- retain the portable promotion conformance tests for successful nested copy, - source preservation, existing destination, cleanup, symlink rejection, and - install collision; -- add platform-specific no-replace tests that run on their native CI platform; -- cross-compile the fileops tests for Linux, Darwin, and Windows; -- confirm the unsupported-platform implementation returns its capability error - before creating a temporary sibling; and -- run `go test ./internal/fileops`, `go test ./...`, `go vet ./...`, and - `go build ./cmd/narratio` on the development platform. - -Exit criteria: - -- Directory promotion is functional and no-replace on Linux, macOS, and - Windows rather than merely compiling there. -- Unsupported platforms fail before expensive copying and have an explicit - documented boundary. -- All available native and cross-compilation checks pass. - -## Stage 14: Reconcile Documentation And Perform Final Remediation Validation - -Make current-behavior documentation match the corrected implementation and -close the remediation effort only after end-to-end verification. - -Implementation: - -1. Correct `docs/internal/stage-extract.md` to say that omitted or disabled - Notarius explicitly self-skips with `notarius_disabled`; do not describe the - manifest result as succeeded. -2. Replace the nonexistent `internal/stage/extract_resume_test.go` reference with - the actual focused test owner, or create that file only if tests were - intentionally reorganized during remediation. -3. Update operations, publish, and extraction documentation to state that: - - changed extraction outcomes stale affected downstream stages; - - repeated identical disabled self-skip does not cause perpetual reruns; - - the run-local Notarius bundle is excluded from run-record upload; and - - only explicit configured extraction lanes are published. -4. Update platform-support documentation from Stage 13 and troubleshooting for - unsupported atomic promotion. -5. Reconcile `docs/roadmap/notarius-extract-stage.md` with the remediated current - state. Mark the feature complete only if every original and remediation - acceptance criterion is satisfied. -6. Recheck maintained examples, internal links, stage inventories, command - examples, field names, schema identities, defaults, and paths against code. -7. Keep roadmap history concise; do not restore the former detailed completed - Stage 1-9 instructions to this file. +1. Resolve the CI substrate and native runner labels in the open question below + before editing workflow files. Follow the repository host's established + workflow location and syntax; do not invent runner labels that cannot run. +2. Add checked-in CI jobs that execute on native Linux, macOS, and Windows + runners. Each native job must run at least: + - `go test -count=1 ./internal/fileops`; + - the platform-specific no-replace test; and + - the portable promotion conformance tests, including successful promotion, + collision preservation, cleanup, and the Stage 15 race-safe traversal + cases that apply on that platform. +3. Ensure the jobs exercise the real platform implementation rather than a + mocked rename function. A destination created before installation must win, + and the source and destination contents must prove that no replacement + occurred. +4. On Windows, ensure successful promotion exercises both `MoveFileEx` and the + directory-sync path. Do not treat permission or ordinary I/O failures as + unsupported-operation success. +5. Retain deterministic cross-compilation of the fileops test binary for Linux, + Darwin, and Windows. Write compiled test binaries to a temporary or CI + artifact directory, never the repository root. +6. Keep unsupported-platform compilation coverage, but do not misrepresent an + unexecuted cross-compiled test as native validation. +7. Record any required runner-specific limitation in this stage and in the + canonical contributor/CI documentation. Do not weaken production semantics + merely to accommodate an inadequately provisioned runner. Validation: -- run focused lifecycle, adapter, fileops, catalog, analyze, publish, restore, - and operator tests without live external services; -- run focused race tests covering the changed extraction, artifact, publish, - fileops, and runner paths; -- run `go test -count=1 ./...`; -- run `go vet ./...`; -- run `go build ./cmd/narratio`; -- run the Stage 13 cross-compilation checks; -- run `git diff --check`; and -- confirm no secrets, private campaign content, generated binaries, or test - artifacts were added to the repository. +- obtain a successful native Linux job; +- obtain a successful native macOS job; +- obtain a successful native Windows job; +- retain successful Linux, Darwin, and Windows cross-compilation; and +- confirm CI does not leave generated test binaries or application binaries in + the worktree or commit them as artifacts of the source tree. Exit criteria: -- All post-implementation review findings are corrected or explicitly bounded - by the documented platform contract. -- Current documentation accurately describes stage outcomes, invalidation, - publication, adapter compatibility, and supported platforms. -- Every progress row is `Complete`, the repository-wide validation suite passes, - and the target feature roadmap can truthfully remain complete. +- Atomic no-replace promotion and its directory-sync behavior execute + successfully on native Linux, macOS, and Windows. +- The checked-in CI configuration will rerun those tests on future changes. +- Stage 17 is not marked complete based solely on Linux execution and + cross-compilation. + +## Stage 18: Reconcile Security Documentation And Perform Final Validation + +Close the remediation only after the corrected filesystem and manifest +invariants are implemented and native platform evidence exists. + +Implementation: + +1. Reconcile `docs/roadmap/notarius-extract-stage.md` with the Stage 15 source + traversal mechanism and Stage 16 current-result semantics. Keep normative + security and manifest guarantees in their existing canonical owners. +2. Update focused internal or contributor documentation only where the new + mechanism or CI workflow changes maintained implementation guidance. Do not + duplicate volatile platform commands across multiple documents. +3. Recheck the extraction, workspace, manifest, operations, troubleshooting, + integration, and testing documentation for contradictions introduced by the + remediation. +4. Keep completed roadmap history concise. Mark the target feature roadmap and + this implementation plan complete only after all Stage 15 through 17 exit + criteria are satisfied. +5. Confirm that no secrets, private campaign material, generated binaries, + platform test binaries, or temporary bundle trees were added to the + repository. + +Validation: + +- run focused fileops, manifest, runner lifecycle, extraction, adapter, + artifact-catalog, analyze, publish, restore, and operator tests; +- run focused race tests covering fileops, manifest transitions, runner + lifecycle, extraction, artifacts, and publication; +- run `go test -count=1 ./...`; +- run `go vet ./...`; +- build `cmd/narratio` with `-o` targeting a temporary directory; +- rerun Linux, Darwin, and Windows fileops test cross-compilation; +- confirm the native CI jobs from Stage 17 are successful; +- run `git diff --check`; and +- verify the worktree contains no generated validation artifacts. + +Exit criteria: + +- The source traversal and current-manifest replacement gaps are closed by + focused regression tests. +- Native filesystem behavior substantiates the documented Linux, macOS, and + Windows support boundary. +- Current documentation matches the remediated implementation. +- Every progress row is `Complete`, the repository-wide validation suite + passes, and the target feature roadmap can truthfully remain complete. ## Open Questions -None. The remediation stages adopt the long-term-maintainable defaults: general -runner invalidation semantics, explicit-only bundle publication, exact current -Notarius management-file semantics, and atomic promotion support on Linux, -macOS, and Windows with an explicit early failure elsewhere. +### Which CI system and native runner labels are authoritative for this repository? + +**Recommended approach:** Use Gitea Actions with a checked-in workflow under +`.gitea/workflows/`, provided the repository owner supplies or confirms native +Linux, macOS, and Windows runner labels. The repository is hosted on Gitea, so +keeping the workflow with the authoritative repository minimizes mirrored +configuration and makes the platform contract visible beside the code. Stage +17 must remain pending until all three native jobs have actually run. + +**Viable alternative:** Use an existing external CI service with real native +capacity, such as a maintained GitHub Actions mirror or Buildkite installation, +and check its workflow or pipeline definition into the conventional repository +location. This is appropriate when that service already owns release gating or +when native macOS and Windows runners are unavailable in Gitea. The alternative +must still produce repeatable native results for all three platforms; +cross-compilation or an undocumented one-time manual run is not sufficient. diff --git a/internal/fileops/directory.go b/internal/fileops/directory.go index c01ff47..695e1c8 100644 --- a/internal/fileops/directory.go +++ b/internal/fileops/directory.go @@ -30,6 +30,19 @@ func PromoteDirectory(src, dst string) error { } func promoteDirectory(src, dst string, install func(string, string) error) error { + return promoteDirectoryWithHooks(src, dst, install, sourceTraversalHooks{}) +} + +type sourceTraversalHooks struct { + afterRootInspect func() + afterEntryInspect func(string) +} + +func promoteDirectoryWithHooks( + src, dst string, + install func(string, string) error, + hooks sourceTraversalHooks, +) error { if strings.TrimSpace(src) == "" || strings.TrimSpace(dst) == "" { return fmt.Errorf("source and destination directory paths are required") } @@ -76,7 +89,13 @@ func promoteDirectory(src, dst string, install func(string, string) error) error } }() - if err := copyRegularTree(src, temporary); err != nil { + sourceRoot, err := openVerifiedSourceRoot(src, sourceInfo, hooks) + if err != nil { + return err + } + defer func() { _ = sourceRoot.Close() }() + + if err := copyRegularTree(sourceRoot, src, temporary, hooks); err != nil { return err } if err := os.Chmod(temporary, promotedDirectoryMode); err != nil { @@ -96,62 +115,153 @@ func promoteDirectory(src, dst string, install func(string, string) error) error return nil } -func copyRegularTree(src, dst string) error { - entries, err := os.ReadDir(src) +func openVerifiedSourceRoot(path string, inspected os.FileInfo, hooks sourceTraversalHooks) (*os.Root, error) { + if hooks.afterRootInspect != nil { + hooks.afterRootInspect() + } + + root, err := os.OpenRoot(path) if err != nil { - return fmt.Errorf("read source directory %q: %w", src, err) + return nil, fmt.Errorf("open source directory %q: %w", path, err) + } + verified := false + defer func() { + if !verified { + _ = root.Close() + } + }() + + opened, err := root.Stat(".") + if err != nil { + return nil, fmt.Errorf("inspect opened source directory %q: %w", path, err) + } + if !opened.IsDir() || !os.SameFile(inspected, opened) { + return nil, fmt.Errorf("source directory %q changed while being opened", path) + } + current, err := os.Lstat(path) + if err != nil { + return nil, fmt.Errorf("reinspect source directory %q: %w", path, err) + } + if current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { + return nil, fmt.Errorf("source directory %q changed while being opened", path) + } + + verified = true + return root, nil +} + +func copyRegularTree(src *os.Root, sourcePath, dst string, hooks sourceTraversalHooks) error { + directory, err := src.Open(".") + if err != nil { + return fmt.Errorf("open source directory %q for traversal: %w", sourcePath, err) + } + defer func() { _ = directory.Close() }() + + entries, err := directory.ReadDir(-1) + if err != nil { + return fmt.Errorf("read source directory %q: %w", sourcePath, err) } sort.Slice(entries, func(i, j int) bool { return entries[i].Name() < entries[j].Name() }) for _, entry := range entries { - sourcePath := filepath.Join(src, entry.Name()) + entryPath := filepath.Join(sourcePath, entry.Name()) destinationPath := filepath.Join(dst, entry.Name()) - info, err := os.Lstat(sourcePath) + info, err := src.Lstat(entry.Name()) if err != nil { - return fmt.Errorf("inspect source entry %q: %w", sourcePath, err) + return fmt.Errorf("inspect source entry %q: %w", entryPath, err) } switch { case info.Mode().IsRegular(): - if err := copyRegularFile(sourcePath, destinationPath, info); err != nil { + if hooks.afterEntryInspect != nil { + hooks.afterEntryInspect(entryPath) + } + if err := copyRegularFile(src, entry.Name(), entryPath, destinationPath, info); err != nil { return err } case info.IsDir(): - if err := os.Mkdir(destinationPath, promotedDirectoryMode); err != nil { - return fmt.Errorf("create destination directory %q: %w", destinationPath, err) + if hooks.afterEntryInspect != nil { + hooks.afterEntryInspect(entryPath) } - if err := copyRegularTree(sourcePath, destinationPath); err != nil { + if err := copyRegularDirectory(src, entry.Name(), entryPath, destinationPath, info, hooks); err != nil { return err } - if err := os.Chmod(destinationPath, promotedDirectoryMode); err != nil { - return fmt.Errorf("set destination directory permissions %q: %w", destinationPath, err) - } - if err := syncDirectory(destinationPath); err != nil { - return fmt.Errorf("sync destination directory %q: %w", destinationPath, err) - } default: - return fmt.Errorf("source entry %q has unsupported file type %s", sourcePath, info.Mode().Type()) + return fmt.Errorf("source entry %q has unsupported file type %s", entryPath, info.Mode().Type()) } } return nil } -func copyRegularFile(src, dst string, inspected os.FileInfo) error { - in, err := os.Open(src) +func copyRegularDirectory( + parent *os.Root, + name, sourcePath, dst string, + inspected os.FileInfo, + hooks sourceTraversalHooks, +) error { + child, err := parent.OpenRoot(name) if err != nil { - return fmt.Errorf("open source file %q: %w", src, err) + return fmt.Errorf("open source directory %q: %w", sourcePath, err) + } + defer func() { _ = child.Close() }() + + opened, err := child.Stat(".") + if err != nil { + return fmt.Errorf("inspect opened source directory %q: %w", sourcePath, err) + } + if !opened.IsDir() || !os.SameFile(inspected, opened) { + return fmt.Errorf("source directory %q changed while being copied", sourcePath) + } + current, err := parent.Lstat(name) + if err != nil { + return fmt.Errorf("reinspect source directory %q: %w", sourcePath, err) + } + if current.Mode()&os.ModeSymlink != 0 || !current.IsDir() || !os.SameFile(opened, current) { + return fmt.Errorf("source directory %q changed while being copied", sourcePath) + } + + if err := os.Mkdir(dst, promotedDirectoryMode); err != nil { + return fmt.Errorf("create destination directory %q: %w", dst, err) + } + if err := copyRegularTree(child, sourcePath, dst, hooks); err != nil { + return err + } + if err := os.Chmod(dst, promotedDirectoryMode); err != nil { + return fmt.Errorf("set destination directory permissions %q: %w", dst, err) + } + if err := syncDirectory(dst); err != nil { + return fmt.Errorf("sync destination directory %q: %w", dst, err) + } + return nil +} + +func copyRegularFile( + root *os.Root, + name, sourcePath, dst string, + inspected os.FileInfo, +) error { + in, err := root.Open(name) + if err != nil { + return fmt.Errorf("open source file %q: %w", sourcePath, err) } defer func() { _ = in.Close() }() - openedInfo, err := in.Stat() + opened, err := in.Stat() if err != nil { - return fmt.Errorf("inspect opened source file %q: %w", src, err) + return fmt.Errorf("inspect opened source file %q: %w", sourcePath, err) } - if !openedInfo.Mode().IsRegular() || !os.SameFile(inspected, openedInfo) { - return fmt.Errorf("source file %q changed while being copied", src) + if !opened.Mode().IsRegular() || !os.SameFile(inspected, opened) { + return fmt.Errorf("source file %q changed while being copied", sourcePath) + } + current, err := root.Lstat(name) + if err != nil { + return fmt.Errorf("reinspect source file %q: %w", sourcePath, err) + } + if current.Mode()&os.ModeSymlink != 0 || !current.Mode().IsRegular() || !os.SameFile(opened, current) { + return fmt.Errorf("source file %q changed while being copied", sourcePath) } out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, promotedFileMode) @@ -166,7 +276,7 @@ func copyRegularFile(src, dst string, inspected os.FileInfo) error { }() if _, err := io.Copy(out, in); err != nil { - return fmt.Errorf("copy source file %q: %w", src, err) + return fmt.Errorf("copy source file %q: %w", sourcePath, err) } if err := out.Chmod(promotedFileMode); err != nil { return fmt.Errorf("set destination file permissions %q: %w", dst, err) diff --git a/internal/fileops/directory_test.go b/internal/fileops/directory_test.go index 7f35330..5d95673 100644 --- a/internal/fileops/directory_test.go +++ b/internal/fileops/directory_test.go @@ -159,6 +159,151 @@ func TestPromoteDirectoryRejectsSymlinksWithoutFollowingThem(t *testing.T) { assertFileBytes(t, filepath.Join(externalDirectory, "secret.txt"), []byte("secret")) } +func TestPromoteDirectoryRejectsSymlinkSourceRoot(t *testing.T) { + root := t.TempDir() + target := filepath.Join(root, "target") + src := filepath.Join(root, "source") + dst := filepath.Join(root, "destination") + mustWriteFile(t, filepath.Join(target, "value.txt"), []byte("outside"), 0o644) + if err := os.Symlink(target, src); err != nil { + t.Skipf("Symlink() unavailable: %v", err) + } + + if err := PromoteDirectory(src, dst); err == nil { + t.Fatal("PromoteDirectory() error = nil, want source-root symlink rejection") + } + assertFileBytes(t, filepath.Join(target, "value.txt"), []byte("outside")) + assertPathMissing(t, dst) + assertNoMatchingTempDirectories(t, root, ".destination.tmp-") +} + +func TestPromoteDirectoryRejectsSourceRootReplacementBeforeOpen(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "source") + preserved := filepath.Join(root, "source-preserved") + replacement := filepath.Join(root, "replacement") + dst := filepath.Join(root, "destination") + mustWriteFile(t, filepath.Join(src, "value.txt"), []byte("original"), 0o644) + mustWriteFile(t, filepath.Join(replacement, "value.txt"), []byte("replacement"), 0o644) + + err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ + afterRootInspect: func() { + if err := os.Rename(src, preserved); err != nil { + t.Fatalf("Rename(original source) error = %v", err) + } + if err := os.Rename(replacement, src); err != nil { + t.Fatalf("Rename(replacement source) error = %v", err) + } + }, + }) + if err == nil { + t.Fatal("promoteDirectoryWithHooks() error = nil, want source identity failure") + } + assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) + assertFileBytes(t, filepath.Join(src, "value.txt"), []byte("replacement")) + assertPathMissing(t, dst) + assertNoMatchingTempDirectories(t, root, ".destination.tmp-") +} + +func TestPromoteDirectoryRejectsInspectedDirectorySymlinkReplacement(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "source") + child := filepath.Join(src, "child") + preserved := filepath.Join(src, "child-preserved") + outside := filepath.Join(root, "outside") + dst := filepath.Join(root, "destination") + mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("original"), 0o644) + mustWriteFile(t, filepath.Join(outside, "sentinel.txt"), []byte("outside"), 0o644) + + replaced := false + err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ + afterEntryInspect: func(path string) { + if replaced || path != child { + return + } + replaced = true + if err := os.Rename(child, preserved); err != nil { + t.Fatalf("Rename(inspected child) error = %v", err) + } + if err := os.Symlink(filepath.Join("..", "outside"), child); err != nil { + t.Skipf("Symlink() unavailable: %v", err) + } + if err := os.Mkdir(dst, 0o755); err != nil { + t.Fatalf("Mkdir(concurrent destination) error = %v", err) + } + mustWriteFile(t, filepath.Join(dst, "value.txt"), []byte("concurrent"), 0o644) + }, + }) + if err == nil { + t.Fatal("promoteDirectoryWithHooks() error = nil, want symlink replacement failure") + } + assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) + assertFileBytes(t, filepath.Join(outside, "sentinel.txt"), []byte("outside")) + assertFileBytes(t, filepath.Join(dst, "value.txt"), []byte("concurrent")) + assertPathMissing(t, filepath.Join(dst, "sentinel.txt")) + assertNoMatchingTempDirectories(t, root, ".destination.tmp-") +} + +func TestPromoteDirectoryRejectsInspectedFileIdentityMismatch(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "source") + file := filepath.Join(src, "value.txt") + preserved := filepath.Join(src, "value-preserved.txt") + dst := filepath.Join(root, "destination") + mustWriteFile(t, file, []byte("original"), 0o644) + + replaced := false + err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ + afterEntryInspect: func(path string) { + if replaced || path != file { + return + } + replaced = true + if err := os.Rename(file, preserved); err != nil { + t.Fatalf("Rename(inspected file) error = %v", err) + } + mustWriteFile(t, file, []byte("replacement"), 0o644) + }, + }) + if err == nil { + t.Fatal("promoteDirectoryWithHooks() error = nil, want file identity failure") + } + assertFileBytes(t, preserved, []byte("original")) + assertFileBytes(t, file, []byte("replacement")) + assertPathMissing(t, dst) + assertNoMatchingTempDirectories(t, root, ".destination.tmp-") +} + +func TestPromoteDirectoryRejectsInspectedDirectoryIdentityMismatch(t *testing.T) { + root := t.TempDir() + src := filepath.Join(root, "source") + child := filepath.Join(src, "child") + preserved := filepath.Join(src, "child-preserved") + dst := filepath.Join(root, "destination") + mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("original"), 0o644) + + replaced := false + err := promoteDirectoryWithHooks(src, dst, renameDirectoryNoReplace, sourceTraversalHooks{ + afterEntryInspect: func(path string) { + if replaced || path != child { + return + } + replaced = true + if err := os.Rename(child, preserved); err != nil { + t.Fatalf("Rename(inspected directory) error = %v", err) + } + mustWriteFile(t, filepath.Join(child, "value.txt"), []byte("replacement"), 0o644) + }, + }) + if err == nil { + t.Fatal("promoteDirectoryWithHooks() error = nil, want directory identity failure") + } + assertFileBytes(t, filepath.Join(preserved, "value.txt"), []byte("original")) + assertFileBytes(t, filepath.Join(child, "value.txt"), []byte("replacement")) + assertPathMissing(t, dst) + assertNoMatchingTempDirectories(t, root, ".destination.tmp-") +} + func TestPromoteDirectoryDoesNotReplaceDestinationCreatedBeforeInstall(t *testing.T) { root := t.TempDir() src := filepath.Join(root, "source") @@ -217,6 +362,13 @@ func assertFileBytes(t *testing.T, path string, want []byte) { } } +func assertPathMissing(t *testing.T, path string) { + t.Helper() + if _, err := os.Lstat(path); !os.IsNotExist(err) { + t.Fatalf("Lstat(%q) error = %v, want not exist", path, err) + } +} + func treeLayout(t *testing.T, root string) []string { t.Helper() var layout []string