Repository navigation
Fail closed when no witness is configured - #40
Conversation
run_turn and python -m runtime do not call the provider unless a witness sends the call. WICKET_ALLOW_DIRECT_CALLS=1 is the local-dev opt-out: off by default, and it warns on stderr. Gossip derives an https_request and sends it only through the witness after a matching allow; unknown shapes are denied and not sent. Co-authored-by: Jon Halstead <warheart1984@gmail.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 60a18cf5b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| summary = signed_gossip_summary() | ||
| body = json.dumps(summary).encode("utf-8") |
There was a problem hiding this comment.
When the policy requires approval for writes, the first invocation binds this body into the proposal hash, but signed_gossip_summary() includes the current timestamp and its signature. A human-approved retry in a later second therefore produces a different call_digest and proposal hash, so ApprovalStore.claim() cannot find the approval and gossip remains awaiting approval indefinitely. Preserve the pending signed body for the retry, or otherwise keep the approval-bound request stable.
Useful? React with 👍 / 👎.
| def direct_calls_allowed() -> bool: | ||
| """True only when the local-dev opt-out is explicitly set to ``1``. Unset is off.""" | ||
| return os.environ.get(DIRECT_CALLS_ENV, "").strip() == "1" |
There was a problem hiding this comment.
The documented fail-closed contract says every value other than exactly 1 leaves direct calls disabled, but stripping whitespace also enables values such as " 1" or "1\n". A value imported from a file or secret with a trailing newline can therefore unexpectedly bypass the witness; compare the raw environment value to "1" instead.
Useful? React with 👍 / 👎.
| else: | ||
| result = {"peer_id": peer_id, "status": "sent"} | ||
| outcome = "completed" |
There was a problem hiding this comment.
When a peer or intermediary returns an HTTP error such as 401 or 500, dispatch_https() still returns the response body and marks the witness execution completed, because http.client does not raise for HTTP status codes. This branch consequently reports status: "sent" and records a completed outcome, whereas the previous urlopen path surfaced those responses as errors. The witness response needs to carry and validate the HTTP status before gossip is classified as completed.
Useful? React with 👍 / 👎.
What this is
The witness requirement is on by default for
run_turn,python -m runtime, and gossip. If no witness is configured, those calls are not sent. This does not prove observed effect.What a reviewer can check
run_turnandpython -m runtimedo not call the provider when no witness is configured. The kernel may still record a decision. On that path the proposal is the runtime's own description (effectread, target the provider name), not a derivedcall_digest, and there is no outcome because nothing was sent.WICKET_ALLOW_DIRECT_CALLSsends only when it is exactly1. It prints a warning on stderr every time. It is not in any deploy unit. It does not let gossip skip an allow. Unset is off.https_request, derives effect, target, andcall_digest, and the witness sends it only after a matching allow. No policy or no witness means the peer is not contacted. An unknown shape (an IPv6 peer) isUNKNOWN_CALL_SHAPEand is not sent.Tests
run_turnwithout a witness send nothing.1warns.What this does not prove
state_ref, retries, heartbeat, and a two-account witness probe.NOVA_ICK_POLICYandNOVA_ICK_SERVICEunset still uses the provider client. That kernel gate remains opt-in. The local-dev opt-out does not derive a digest.riskandactionare still the caller's claim. The clock is unproven. A compromised witness can sign a false execution. The authorization header is presence-only.User-Agentis sent and not hashed. Streamed known-shape calls stay refused.Limits are stated in
docs/executor-mode.md.