feat(agents): enforce trusted heartbeat source preflight - #5602
feat(agents): enforce trusted heartbeat source preflight#5602Greg-Moskalenko wants to merge 2 commits into
Conversation
Co-authored-by: Greg Moskalenko <greg1mosk@gmail.com> Signed-off-by: Greg Moskalenko <greg1mosk@gmail.com>
![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: 939b524723
ℹ️ 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".
| use std::os::unix::ffi::OsStrExt; | ||
|
|
||
| type Acl = *mut c_void; | ||
| unsafe extern "C" { |
There was a problem hiding this comment.
This introduces an unsafe extern block and several unsafe calls for ACL inspection, despite the repository’s explicit prohibition on any unsafe code. Replace this implementation with a safe wrapper or command-based ACL inspection so the macOS heartbeat verification path complies with the project’s required safety invariant.
AGENTS.md reference: AGENTS.md:L114-L116
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8589050c9. The macOS verifier no longer uses FFI or any unsafe code. It now performs fail-closed ACL inspection through /bin/ls -lde with a cleared environment, bounded output, strict status/stderr/UTF-8 checks, and an ACL regression test. The full just ci gate passes on this exact head.
| }; | ||
|
|
||
| // ── Pre-Phase 2: validate provider config BEFORE any side effects ──────── | ||
| input.validate_heartbeat_preflight(&pubkey)?; |
There was a problem hiding this comment.
When heartbeatPreflight is supplied during creation, the target pubkey is generated immediately above this call and is never exposed to the caller, but policy validation requires target_agent_pubkey to equal that newly generated value. A caller therefore cannot prepare a valid policy in advance, and retrying after failure generates another key, making the new create-time designation path effectively unusable; allocate or expose the key before accepting the policy, or require designation through the subsequent update path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 8589050c9. Create requests now reject a heartbeat designation before any side effect, the TypeScript create surface no longer exposes that field, and designation is required through update_managed_agent after creation returns the agent pubkey. The new boundary test and the full just ci gate pass on this exact head.
Require heartbeat designation through the post-create update path once the agent pubkey exists. Replace the macOS ACL FFI with fail-closed system ACL inspection so the trusted harness path contains no unsafe Rust. Co-authored-by: Greg Moskalenko <greg1mosk@gmail.com> Signed-off-by: Greg Moskalenko <greg1mosk@gmail.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Current head 8589050 addresses both automated review findings and passes the full local |
1 participant
Summary
The bug this addresses is structural: prompt text can ask an agent to call every source, but it cannot prove those calls happened before the agent reports a no-op. This change moves that decision to a pre-agent runtime gate and makes incomplete source coverage suppress the model turn.
Related issue
None found. I searched open issues and PRs for heartbeat preflight and source attestation. #4983 is related scoped native-access/delivery work, but it does not provide this source-receipt gate; #5533 concerns heartbeat thread guidance rather than source execution.
Key safety properties
Testing
just ci— passed on exact head8589050c9c1285bfe1ef0a90b79650df9ce7bb7dgit diff --check— passedManual testing / screenshots
No new end-user UI is introduced, so screenshots are not applicable. A production acceptance run still requires the separately deployed trusted gateway, source credentials/helpers, and fresh signed source receipts; this PR deliberately fails closed until those are present.
Deferred follow-up