fix: integrate issue 401 ownership repairs - #16
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6702edb1d4
ℹ️ 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".
| @@ -2561,7 +2628,8 @@ def _drop_entry() -> None: | |||
|
|
|||
| def check_all_command_guards(command: str, env_type: str, | |||
| approval_callback=None, | |||
| has_host_access: bool = False) -> dict: | |||
| has_host_access: bool = False, | |||
| require_explicit_authorization: bool = False) -> dict: | |||
There was a problem hiding this comment.
Propagate explicit authorization to deferred goal commands
The new option is never enabled by production code: repo-wide search finds only the direct tests passing require_explicit_authorization=True, while tools/terminal_tool.py calls the guard without it. Consequently, a synthetic /goal continuation that issues a dangerous terminal command still accepts once or always, so the advertised session-only automatic-goal policy has no runtime effect. Mark goal-continuation execution in context and propagate that state through the terminal wrapper to this argument.
AGENTS.md reference: AGENTS.md:L122-L125
Useful? React with 👍 / 👎.
| @@ -2879,6 +2964,20 @@ def check_all_command_guards(command: str, env_type: str, | |||
| "deny_reason": deny_reason, | |||
| } | |||
|
|
|||
| if require_explicit_authorization and choice != "session": | |||
There was a problem hiding this comment.
Enforce explicit authorization before allowlist bypasses
This check runs only after a fresh prompt, but a previously permanent-approved pattern is filtered by is_approved() (and an exact/glob permanent allowlist returns even earlier at lines 2684-2685), leaving warnings empty and returning approved before reaching this branch. Thus, once the deferred path is wired, a prior always approval authorizes an automatic goal command even though this branch explicitly says permanent approval must not do so; distinguish session authorization before applying permanent allowlist shortcuts. The added mocked prompt tests do not exercise this security-boundary path.
AGENTS.md reference: AGENTS.md:L84-L87
Useful? React with 👍 / 👎.
9ca8984 to
09bd92e
Compare
09bd92e to
df9408c
Compare
Summary
Fresh current-main rebuild for the accepted
cermm/wc-infrastructureNousResearch#401 child deltas:mainmainmainThis intentionally does not reuse stale conflicting PR #11. This PR supersedes #15, which used the same source tree but had automation-only commit attribution that failed the contributor check.
Exact candidate
d4b6ecf622c44a96eff2a79b2f0c904e678b2f386702edb1d4971fc0e411096e7b5e82f7dd6a385e6fc554824c18735804c82941bac25e1803bcde9a4a9f21d50c9ad82295527bd17230c89238bdcf51→ identical tree/diff to this headVerification
Local result: no source drift vs
4a9f21d50c9ad82295527bd17230c89238bdcf51;97 passed;compileall-ok; diff check clean.Independent exact-head review: PASS/PASS on
6702edb1d4971fc0e411096e7b5e82f7dd6a385e.Refs cermm/wc-infrastructure#469
Refs cermm/wc-infrastructure#475
Refs cermm/wc-infrastructure#477
Refs cermm/wc-infrastructure#401