spec: resolver-trusted-parent - #276
Conversation
Deploying ystack with
|
| Latest commit: |
95cdf5b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://82b6cf39.fabrica-6yx.pages.dev |
| Branch Preview URL: | https://ystack-spec-resolver-trusted.fabrica-6yx.pages.dev |
Codex reviewer (cross-vendor, read-only)Reviewed-head: bd232b1 Posted verbatim by The spec currently contradicts the accepted no-network and human-merge constraints and contains an unimplementable parent-side helper provenance requirement. These issues should be fixed before the artifact is accepted as the basis for implementation. Full review comments:
|
…table helper provenance)
Codex reviewer (cross-vendor, read-only)Reviewed-head: ebbef0f Posted verbatim by The new high-risk spec contains contradictions and test gaps that would authorize a leaky or insufficiently bound trusted launcher. These should be corrected before accepting the artifact for planning. Full review comments:
|
…g with stated residual; direct-parent jq test; narrowed read claim)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 5a18ff3 Posted verbatim by The added spec has actionable issues in the accepted size bound and in requirements that would let the implementation or test proof miss declared boundary guarantees. These should be corrected before the spec is accepted as the basis for planning. Full review comments:
|
…mporaries inside the run dir; cleanup proved after mktemp)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 52e10fd Posted verbatim by The spec leaves gaps in the trusted-code provenance and signal handling for a high-risk launch boundary, and its required test does not actually cover the parent refusals it claims. These issues should be fixed before accepting the artifact. Full review comments:
|
…tion on signals; direct-parent coverage)
Codex reviewer (cross-vendor, read-only)Reviewed-head: eb96ef6 Posted verbatim by The spec contains contradictory parent pinning requirements and permits degraded signal-test proof for newly added security-boundary behavior. These should be corrected before accepting the artifact. Full review comments:
|
…sistently; race-free signal test via SIGSTOP)
Codex reviewer (cross-vendor, read-only)Reviewed-head: a69c86e Posted verbatim by The spec contains internally inconsistent and infeasible test requirements for a high-risk security-boundary component. These should be corrected before accepting the artifact because the downstream plan and proof would otherwise be built on inaccurate requirements. Full review comments:
|
… allowlist; reachable overlong case)
Codex reviewer (cross-vendor, read-only)Reviewed-head: a8a9374 Posted verbatim by The spec adds a materially inaccurate read-boundary claim and widens the write surface beyond the accepted intent. These should be corrected before the artifact is accepted and used to drive implementation. Full review comments:
|
…ry inside the caller's output root)
Codex reviewer (cross-vendor, read-only)Reviewed-head: b568fc0 Posted verbatim by The spec leaves security-boundary behavior under-specified in ways that would either write into an unvalidated output root or make the proposed environment tests/provenance claims unreliable. These should be fixed before accepting the artifact as the basis for implementation. Full review comments:
|
… vars only via the entry; compile under env -i)
Codex reviewer (cross-vendor, read-only)Reviewed-head: d8da6ec Posted verbatim by The spec has blocking contradictions and security-boundary gaps that would either make the planned test fail or permit behavior the spec explicitly promises to prevent. These should be resolved before using this artifact as the implementation contract. Full review comments:
|
… before pins; fd-relative sandbox writes)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 0540b47 Posted verbatim by The spec has security-boundary gaps around supported Darwin writes, same-uid replacement of the run directory path, and the entry script's clean-path handoff. These should be resolved before treating the artifact as an accepted basis for implementation. Full review comments:
|
…dual stated; marker branch re-scrubs)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 5619bfc Posted verbatim by The spec leaves a path re-resolution hole in the parent output handling and explicitly permits Darwin writes outside the output root despite the accepted intent forbidding them. These issues should be resolved before using the spec as the basis for the high-risk implementation. Full review comments:
|
…n shim in the shipped path)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 9788015 Posted verbatim by The spec has two actionable security-boundary issues in the signal forwarding loop and inherited file-descriptor handling. These could lead the implementation plan to preserve a pid-reuse signal hazard and fd leaks into pre-resolver helper processes. Full review comments:
|
…able says running; inherited fds closed at startup)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 67bb175 Posted verbatim by The spec leaves a signal window that can orphan resolver descendants, and its descriptor-close requirement is placed after earlier processes can already inherit caller fds. These are correctness/security issues in the accepted design before implementation starts. Full review comments:
|
…leanup; fd close precedes scrub and re-exec)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 8c26952 Posted verbatim by The spec leaves two security-boundary details wrong or underspecified: the signal handler can mutate the caller's stderr flags, and direct parent helper probes can inherit caller-controlled environment. These should be fixed before using the spec as implementation guidance. Full review comments:
|
…s; parent helpers exec with a fixed envp)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 9c1b9e3 Posted verbatim by The spec leaves two signal-path stderr handling bugs that can break the stated cleanup/exit contracts under realistic pipe and signal timing scenarios. Full review comments:
|
… block; entry diagnostic omitted where a write could block)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 5a0b87c Posted verbatim by The spec would lead to a shell entry that can close Bash's own script descriptor, and it leaves unsafe signal-handler behavior underspecified. These are blocking issues for a high-risk launch boundary artifact. Full review comments:
|
…descriptor; handler calls only async-signal-safe functions)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 1a4bb6d Posted verbatim by The spec leaves a security-boundary gap in its descriptor-closing design: a caller-supplied writable descriptor to the entry script can survive into child processes. This contradicts the stated no inherited write handles/no writes outside output-root behavior. Review comment:
|
… relocates its own input; ulimit headroom precondition)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 2388ec6 Posted verbatim by The spec's signal-handling requirements are internally inconsistent and leave a race where the entry may fail to forward or correctly report a signal after the parent starts. Full review comments:
|
…orded signals forwarded before each wait)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 83a581c Posted verbatim by The spec contains two actionable correctness issues: one can lead the planned signal handler to signal a reaped/reused pid, and one makes a required regression test ineffective. These should be fixed before treating the artifact as accepted. Full review comments:
|
…e pid; full-pipe test fills with a blocking writer)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 794611c Posted verbatim by The spec is hash-linked and risk-classified, but it omits the required exact review_size record for this oversized artifact PR. That leaves the manual plan-gate compliance record incomplete. Review comment:
|
Codex reviewer (cross-vendor, read-only)Reviewed-head: fce5309 Posted verbatim by The spec leaves a direct-parent path able to execute an untrusted awk next to a valid jq, and its fd cleanup design can leak high-numbered caller descriptors to helper children. These are security-boundary issues that should be corrected before accepting the patch. Full review comments:
|
… verified by the parent; /dev/fd enumeration; docs name the omission rule)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 7bfc9b6 Posted verbatim by The spec currently authorizes a direct parent path that does not preserve the pinned dependency boundary, and it also specifies a signal diagnostic path that can still hang on blocking terminal/character stderr. These are blocking correctness issues for a high-risk launch-boundary spec. Full review comments:
|
…parent pins all eight runtime files; entry diagnostic only to regular files)
Codex reviewer (cross-vendor, read-only)Reviewed-head: d362560 Posted verbatim by The spec contains a security-boundary overclaim that conflicts with its own later description of a pre-close fork inheriting caller descriptors. Review comment:
|
…ion; no child before the close loop)
Codex reviewer (cross-vendor, read-only)Reviewed-head: a558ffe Posted verbatim by The added spec mandates a descriptor-closing sequence that can silently fail under a plausible inherited-fd state, undermining a stated security boundary. That should be corrected before accepting the artifact. Review comment:
|
…es; limit ladder explained)
Codex reviewer (cross-vendor, read-only)Reviewed-head: ec23df1 Posted verbatim by The added high-risk spec contains internal contradictions in security-relevant requirements. These should be corrected before it is used to drive implementation and tests. Full review comments:
|
…dition and printf allowlist made consistent)
Codex reviewer (cross-vendor, read-only)Reviewed-head: 95cdf5b Posted verbatim by The added spec contains signal-handling requirements that would direct the implementation toward incorrect status handling and contradict its own command-substitution rule. These should be corrected before accepting the artifact as the implementation contract. Full review comments:
|
Tracks #271
G2 spec for the merged intent (G1, PR #272). Frontmatter records intent-blob 0fd28feb8f1de6ce71ccb90e2fce69056ec2ab32 and risk: high (the launch boundary is the resolver's security boundary). Merging this PR accepts both. It does not close the intake issue.
This PR adds only
work/resolver-trusted-parent/spec.md.