Roadmap item 4: Add inactive local Git materializer - #229
Conversation
…cal-git-materializer-v1 # Conflicts: # README.md # RESTORE.md # adapters/local-git-materializer/v1/protocol.jq # ci/required-files.txt
Codex reviewer (cross-vendor, read-only)Reviewed-head: fc92725 Posted verbatim by The main flow is coherent, but failure atomicity is broken when response delivery fails, and two documented source-sanitization states can bypass the current filesystem checks. Full review comments:
|
Deploying ystack with
|
| Latest commit: |
da15ae0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c0b1cb1c.fabrica-6yx.pages.dev |
| Branch Preview URL: | https://codex-local-git-materializer-f7ju.fabrica-6yx.pages.dev |
Codex reviewer (cross-vendor, read-only)Reviewed-head: 2bab371 Posted verbatim by The materializer violates its scratch-write boundary and contains a newline-based allowed-path bypass that can produce a successful but false receipt. It also processes unbounded raw input before applying schema limits. Full review comments:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 52fa387 Posted verbatim by The materializer rejects some inputs accepted by its protocol and does not fully enforce its documented repository-path safety boundary. Both issues affect valid or adversarial repository inputs. Full review comments:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: c143e07 Posted verbatim by The materializer leaves its largest untrusted write unbounded and allows host Git templates to contaminate the supposedly sanitized candidate repository. Both issues undermine the adapter's safety guarantees. Full review comments:
|
|
Construction review cap reached on exact head Operator ruling requested: authorize one strictly bounded final revision on this same PR to add a fixed reachable-object import budget with an over-budget regression test, and initialize from an explicit empty scratch template with a host-template contamination regression test. The PR remains limited to its existing five runtime paths; no authority, activation, credential, network, release, install, or deployment scope changes. After authorization, the same branch will rerun targeted tests, exact CI, and one final independent review before any squash merge. |
|
Operator ruling received directly in the active coordinator session on 2026-09-03: authorize the exact two-item bounded revision proposed in comment 5529174878. This ruling covers only a fixed reachable-object import budget plus regression proof, and an explicit empty Git template plus regression proof, on the same branch and existing five-path PR scope. It does not waive targeted tests, fresh exact CI, final independent review, or publisher receipt. |
Codex reviewer (cross-vendor, read-only)Reviewed-head: e1f9335 Posted verbatim by The implementation makes the protocol's no-change outcome unavailable for the natural empty-patch case. Its path validation also permits severe process amplification for repositories that remain within the advertised import budgets. Full review comments:
|
|
The operator-authorized final review completed on exact head Operator ruling requested: authorize one further strictly bounded correction on this same PR to accept an empty patch by skipping Git apply and prove a canonical no-change result, and replace per-component external path checks with a single bounded validation pass (with amplification regression proof). Keep the existing five-path scope and all inactivity/authority restrictions. Fresh targeted tests, exact CI, and a final independent review remain required before merge. |
|
Operator ruling received directly in the active coordinator session on 2026-09-03: authorize the exact two-item correction proposed in comment 5530340949. This covers only empty-patch no-change support and bounded path validation without per-component external processes, with regression proof, on the same branch and existing five-path scope. It does not waive fresh targeted tests, exact CI, final independent review, or publisher receipt. |
Codex reviewer (cross-vendor, read-only)Reviewed-head: 8eae999 Posted verbatim by The hidden worker command bypasses the adapter's environment-cleaning boundary. The patch also asserts proof for resource-limit behavior that its focused test never exercises. Full review comments:
|
|
The final independent review on exact head Operator ruling requested: authorize one strictly bounded correction on this same PR to guarantee environment sanitization for every worker entry with a hostile exported-function regression, and add focused fixtures that cross the reachable-object-count and tree-listing-byte limits (or narrow any proof wording that cannot be exercised honestly). Keep the existing five-path scope and all inactivity/authority restrictions. Fresh targeted tests, exact CI, and a final independent review remain required before merge. |
|
Standing operator ruling received directly in the active coordinator session on 2026-09-03. For PR #229, the coordinator may resolve all further independent-review Important findings without repeated approval only while the work remains the same local Git materializer concern, stays within the existing five PR paths, and does not expand architecture, authority, permissions, credentials, network, activation, release, install, deployment, or external-write scope. Every new head still requires targeted tests, fresh exact app-15368 CI, and a complete independent exact-head/base review with no unresolved Important before publisher squash merge. Any scope or safety-model change, genuine review disagreement, identity mismatch, or evidence gap restores needs-human. |
Codex reviewer (cross-vendor, read-only)Reviewed-head: 62b39cc Posted verbatim by The materializer's stated input bounds do not bound candidate data written during patch application. A compact, contract-valid input can therefore cause severe disk amplification before the post-apply checks run. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: f25170e Posted verbatim by The materializer can exceed its candidate-write resource bound through copy fan-out and can silently lose valid empty tree entries outside the materialization contract. Both violate core safety and correctness guarantees of the new adapter. Full review comments:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: f085b18 Posted verbatim by Several resource limits are enforced only after unbounded reads or parsing have already occurred. A malicious or concurrently modified local input can therefore exhaust memory, CPU, or scratch space despite the adapter's documented bounds. Full review comments:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: ab26997 Posted verbatim by The materializer rejects a valid class of text patches that replace a directory with a regular file, despite the contract not excluding such transitions. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 3885f5b Posted verbatim by The materializer enforces several limits only after Git has already parsed source-controlled data. Oversized historical objects and packed refs can therefore cause resource exhaustion despite the documented bounds. Full review comments:
|
|
Standing ruling boundary reached. Review comment 5534548633 requires proving every historical commit/tree size before recursive Git traversal. The current five-file shell runtime cannot do that safely: Operator decision requested: authorize the smallest architecture expansion for a separately named bounded object-closure helper, with its own regression tests, durable exception rationale, fixed removal/re-evaluation condition, and manifest/docs updates; or direct us to defer the local materializer runtime and continue the Roadmap with an inactive unsupported forge binding. Until that decision, #229 stays needs-human and will not merge. |
|
Operator authorization received directly in the active coordinator session on 2026-09-04: choose the helper-expansion option from comment 5535343923. Scope expands only by Exception lifecycle: the external invariant is that supported Git CLI traversal parses ancestor commits/trees before the caller can enforce per-object resource limits. Re-evaluate and remove the helper when the supported Git interface provides pre-parse per-object bounds or an accepted sandbox boundary supplies equivalent hard memory/CPU/disk enforcement. The helper must not be copied elsewhere without a separate decision. Existing uncommitted packed-refs work remains part of this same authorized correction. Fresh focused tests, exact CI, independent review, and publisher receipt remain mandatory. |
Codex reviewer (cross-vendor, read-only)Reviewed-head: 2df4dd2 Posted verbatim by The materializer's packed replacement-ref check can be bypassed by valid uppercase hexadecimal object IDs, violating its fail-closed source validation. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 8459a0a Posted verbatim by The helper can either fail under a stricter execution boundary or relax its inherited resource controls. This violates the adapter's sandbox assumptions while it processes untrusted Git data. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 6b6664d Posted verbatim by The path-overlap guard fails for a root source directory, allowing writes within the source boundary before rejection. This breaks an explicit isolation guarantee. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 7239500 Posted verbatim by The materializer's sanitization boundary still trusts and forwards an inherited executable search path. In affected environments this permits arbitrary command execution and invalidates the adapter's safety checks. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: c2d2d4a Posted verbatim by The materializer mishandles an otherwise accepted absolute jq path when it contains shell-splitting characters. This causes a valid invocation to fail and can select the wrong executable. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: c829634 Posted verbatim by The materializer's fail-closed tree validation can silently succeed after scratch write failures, potentially skipping security-relevant subtree checks. This should be corrected before the adapter is considered reliable. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: d214301 Posted verbatim by The materializer's expanded path listing is not covered by its stated 16 MiB tree-scan bound, allowing valid bounded trees to consume substantially more scratch space than promised. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: 6bb997c Posted verbatim by The tree scanner's new expanded-path bound covers blob output but not directory paths appended to its queue. A crafted bounded source tree can therefore cause substantially larger scratch writes than the stated ceiling. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: da15ae0 Posted verbatim by No discrete, actionable correctness defects were identified in the changes relative to the specified merge base. |
Summary
core.forge.materialize-candidate.v2.Scope
Tracks Roadmap item 4 without closing an intake issue. This is the runtime unit after protocol PR #228. The accepted main protocol is unchanged byte-for-byte; this PR adds only runtime code, its focused test, restore entries, and synchronized docs. Default-profile assembly follows after this runtime has a durable main identity.
Safety
The runtime is inactive and local-only. It rejects worktrees, alternates, shallow or partial repositories, replace/graft state, hooks, filters, remotes, unsafe paths, binary patches, symlinks, submodules, and overlapping or non-private boundaries. It receives no credential or network access and cannot push, publish, merge, install, activate a profile, contact a provider, or use a real target during construction.
Targeted proof
The preserved runtime branch was adopted through the operator-approved merge topology. Relative to current main the PR contains exactly five runtime concern paths; it was not rebased or force-pushed.