diff --git a/docs/roadmap/remediation.md b/docs/roadmap/remediation.md new file mode 100644 index 0000000..ba78ee8 --- /dev/null +++ b/docs/roadmap/remediation.md @@ -0,0 +1,108 @@ +# Main-relative companion implementation review + +Reviewed the eight implementation commits from `ae0c94d` through `18af2d2` +against the [implementation plan](implementation.md) and +[feature roadmap](main-relative-inputs.md). + +| Stage | Review result | +| --- | --- | +| 1 | The root facade copies binding and fallback descriptions. Preflight checks active binding scope and static context shape. Concrete overrides and inactive overlays take precedence over shadowed settings. | +| 2 | The acquired main's resolved context feeds underlay acquisition. Derived inputs retain the namespace and containment root, clear the main's physical path, and use receiving-tier host identity. Fallback selection and namespace anchors use the chosen context. | +| 3 | Bound traversal and contextless imports share declaration policy and placement preparation, with source-local diagnostic sites. | +| 4 | Without a base, the loader retains payload and version processing, honors requiredness and the empty named-path exception, and emits no fabricated selection evidence. Root assembly has a second implementation. See Finding 2. | +| 5 | Layered overlays select explicit, main-derived, then fallback context. Lookup base and containment root are separate. Native link handling leaves a gap in containment and duplicate detection. See Finding 1. | +| 6 | The executable example covers discovery, embedded defaults, mounted file and recursive directory imports, transitive imports, whole-record upsert, and the selected overlay. A companion test rejects duplicate peers. | +| 7 | Public and engine tests cover discovery, source and version evidence, optional resources, budgets, and cleanup. The exhaustion and lazy fallback branches need focused regression coverage. See Finding 3. | +| 8 | The configuration contract, public comments, architecture policy, and ADR describe deferred binding. Two supporting descriptions misstate ownership. See Finding 4. | + +## Finding 1 — Native overlay symlinks bypass containment and duplicate checks + +**Priority P2.** The layered branch passes the inherited root and lookup base +to the shared overlay loader. That loader calls `filesystemAccess.canonical`. +For a native filesystem, the function stats the path and returns its lexical +spelling even when the filesystem implements `fs.ReadLinkFS`. The root and +duplicate checks then compare that spelling, while the subsequent read can +follow the link. The same limitation predates this sprint in the shared +loader. Automatic derivation makes it reachable without an explicit overlay +filesystem, and the new contract describes the inherited root as the +containment boundary. [Layered overlay selection](../../internal/engine/load_request.go#L192) +· [Overlay checks](../../internal/engine/selected_overlay.go#L74) +· [Native path lookup](../../internal/engine/filesystem_access.go#L94) +· [Overlay contract](../config.md#selected-overlays) + +Reproduced through `LoadLayered` on Go 1.27.3 using +`NewFilesystem("native", os.DirFS(dir))`, which exposes `fs.ReadLinkFS`. +The main is `scope/main.yml` with a context rooted at `scope`. There is no +explicit overlay context. + +- `scope/overlays/escape.yml` points to `../../outside.yml`. The load + succeeds and takes `count: 9` from outside the root. +- `scope/overlays/repeat.yml` points to `../main.yml`. The load succeeds + and reports the main content a second time under the overlay alias. + +Apply capability-aware link resolution to native overlay targets, using the +existing native link resolver where applicable. Use the resolved path for +containment, repetition, and reading. Check the resolved identity against +already acquired files in the same namespace, including aliases where those +files retain lexical paths. Add focused derived-overlay regressions for the +escape and repeat cases. Preserve root-relative lookup for ordinary and +explicit overlays and document the limit for native filesystems that cannot +expose link targets. This requires no general sandbox guarantee for caller +filesystems. [Native link resolver](../../internal/engine/filesystem_links.go#L227) + +## Finding 2 — Contextless imports duplicate root contribution assembly + +**Priority P3.** Stage 4 calls for sharing the root contribution assembly +and accounting. The new contextless path has its own source charging, +mapping placement, wrapper charging, pending sequence-item scope +construction, and placement reservation. Bound imports perform those same +steps independently. Both paths pass today’s tests, but changes to placement +or accounting now need to be made twice for the same declaring payload. +The initial limit diagnostics already differ in specificity. +[Contextless path](../../internal/engine/contextless_imports.go#L51) +· [Bound import assembly](../../internal/engine/import_acquisition.go#L115) + +Share the preparation and finalization of the declaring root contribution +where the paths have the same semantics. Keep the contextless requiredness +decision and bound traversal separate. Preserve their existing evidence and +the contextless path's lack of filesystem access. Use existing mapping, +sequence-item, and budget checks to verify the extraction. + +## Finding 3 — Exhaustion and lazy fallback lack focused regression coverage + +**Priority P3.** The context-selection test uses an existing fallback when +checking that main derivation wins. It would still pass if the loader +prematurely opened that fallback. The no-base tests use `NoSource()`, and +the discovery integration covers a successful selection or a selected +parse failure. These cases leave the optional-search exhaustion handoff to +both companion and overlay selection unprotected. +[Context selection tests](../../facade_main_relative_test.go#L39) +· [Discovery integration tests](../../facade_main_relative_integration_test.go#L12) + +A disposable public API probe confirmed the current behavior. Exhausted +optional search without context retained the declaring payload with no empty +selection evidence. The same search with a root-only main context loaded a +companion and overlay despite a well-shaped but nonexistent fallback. With +the main context removed, the selected missing fallback failed. Add a small +public regression table for those outcomes. Include an overlay-only failure +case if the shared fixture would otherwise stop at the companion error +before reaching overlay selection. + +## Finding 4 — Supporting documentation misstates binding ownership + +**Priority P3.** The consumer guide locates main-relative bindings in +`LayeredOptions`, but a companion binds through `Input.MainRelative`. +The import internals guide says passing the main context replaces tier-local +identity state. The engine preserves the receiving tier's bookkeeping and +sets the derived host token to that tier's token. Correct these descriptions +and leave the observable rules with the configuration contract. +[Consumer guide](../consumers/README.md#choosing-a-loading-entry-point) +· [Import internals](../internal/imports.md#discovery-and-acquisition) +· [Receiving tier](../../internal/engine/load_request.go#L276) + +## Verification + +The everyday checker and the full race suite passed on Linux with Go 1.27.3. +Disposable public API probes reproduced Finding 1 and checked the exhaustion +and fallback behavior in Finding 3. The review found no additional contract +strengthening beyond the native containment wording discussed in Finding 1.