Clear stale yolo approval state on no-launch reruns - #6868
Conversation
The no-launch session config dir is deliberately reused across runs, but the config writers only ever added the --yolo auto-approval settings and never removed them. After one --yolo --no-launch run, every later run without --yolo kept OpenClaw's tools.exec security=full/ask=off policy plus exec-approvals.json, and OpenCode's permission allow block, so tool execution stayed silently pre-approved. Non-yolo runs now reset that state: OpenClaw drops the exec policy keys and the yolo defaults in exec-approvals.json (approvals OpenClaw itself recorded are kept; the file is removed when only the yolo payload is left), and OpenCode drops the permission block. Launch mode is untouched since it already uses an ephemeral temp dir.
|
@codex review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces state cleanup for OpenClaw and OpenCode configurations when running without the --yolo flag, ensuring that auto-approval settings left behind by previous --yolo runs are correctly stripped. It also adds comprehensive unit tests to verify this behavior. The reviewer identified a potential issue where an unparseable exec-approvals.json file could be silently deleted due to a fallback to an empty dictionary, and suggested checking if the parsed state is not None before proceeding.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| state = _read_json_object(approvals) or {} | ||
| had_defaults = state.pop("defaults", None) is not None | ||
| if set(state) <= {"version"}: | ||
| # Nothing left but our own yolo payload (or unreadable): remove it. | ||
| approvals.unlink() | ||
| typer.echo(f"Removed {approvals}") | ||
| elif had_defaults: | ||
| # Keep approvals OpenClaw itself recorded; only the yolo defaults go. | ||
| _write_private_json(approvals, state) | ||
| typer.echo(f"Updated {approvals}") |
There was a problem hiding this comment.
If _read_json_object(approvals) returns None (e.g., due to a JSON parsing error or OSError), fallback to {} via or {} causes set(state) <= {"version"} to evaluate to True. This results in the silent deletion of exec-approvals.json via approvals.unlink().
Following the design of _read_json_object (which returns None specifically so that callers can leave user-managed files untouched rather than clobbering or deleting them), we should check if state is not None before proceeding, preserving the file if it is unparseable.
| state = _read_json_object(approvals) or {} | |
| had_defaults = state.pop("defaults", None) is not None | |
| if set(state) <= {"version"}: | |
| # Nothing left but our own yolo payload (or unreadable): remove it. | |
| approvals.unlink() | |
| typer.echo(f"Removed {approvals}") | |
| elif had_defaults: | |
| # Keep approvals OpenClaw itself recorded; only the yolo defaults go. | |
| _write_private_json(approvals, state) | |
| typer.echo(f"Updated {approvals}") | |
| state = _read_json_object(approvals) | |
| if state is not None: | |
| had_defaults = state.pop("defaults", None) is not None | |
| if set(state) <= {"version"}: | |
| # Nothing left but our own yolo payload: remove it. | |
| approvals.unlink() | |
| typer.echo(f"Removed {approvals}") | |
| elif had_defaults: | |
| # Keep approvals OpenClaw itself recorded; only the yolo defaults go. | |
| _write_private_json(approvals, state) | |
| typer.echo(f"Updated {approvals}") |
There was a problem hiding this comment.
Fixed in cc60098: an unparseable exec-approvals.json is now left in place (with the cleanup skipped), matching how an unparseable config is handled elsewhere in this file. Test added.
![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: b4ff621301
ℹ️ 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".
| for field in ("host", "security", "ask"): | ||
| exec_policy.pop(field, None) |
There was a problem hiding this comment.
When the reused no-launch config already contains an OpenClaw exec policy that was not written by --yolo (for example a stricter security value or ask: "on"), this loop removes those fields solely because their names overlap with the yolo payload. The surrounding writer otherwise merges and preserves existing config, so a plain unsloth start openclaw --no-launch can silently discard a user's/runtime exec policy; only remove the exact yolo values that this command wrote.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cc60098: each exec policy field is now removed only when it carries the exact value the yolo path writes (host=gateway, security=full, ask=off), so stricter or hand-set policies survive. Test added.
| else: | ||
| # The no-launch config is reused across runs; drop a permission block left by | ||
| # a previous --yolo run so this session prompts again. | ||
| config.pop("permission", None) |
There was a problem hiding this comment.
A no-yolo rerun now deletes any existing OpenCode permission block, not just the auto-allow block written by a previous --yolo run. In the persistent no-launch config this can erase user/session permissions such as denied tools or ask-only policies during an ordinary reconnect, despite the writer's merge-preserve behavior elsewhere; gate the cleanup on the block matching the yolo {edit,bash,webfetch}: allow payload.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cc60098: only edit/bash/webfetch entries whose value is allow are dropped; deny/ask policies and other tools' entries are kept. Test added.
| approvals = path.parent / "exec-approvals.json" | ||
| if approvals.exists(): | ||
| state = _read_json_object(approvals) or {} | ||
| had_defaults = state.pop("defaults", None) is not None |
There was a problem hiding this comment.
OpenClaw's host approvals file stores enforceable defaults, not just the yolo shortcut. In the persistent no-launch state dir, if a user or OpenClaw UI has set non-yolo defaults such as stricter security/ask behavior, this pop removes them on every plain unsloth start openclaw --no-launch and may delete the whole file as stale yolo state; only clear defaults when they match the yolo payload this command wrote.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in cc60098: approvals defaults are now cleared per field and only when they match the yolo payload (security=full, ask=off, askFallback=full); stricter defaults are kept and the file is only removed when nothing but the yolo payload was in it. Test added.
Match each field against the exact value the yolo path writes before removing it, so a stricter exec policy, approvals defaults set by the user or the OpenClaw UI, and deny/ask OpenCode permission entries all survive a plain no-launch rerun. An unparseable exec-approvals.json is left in place, matching how an unparseable config is handled.
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 15aca2ac58
ℹ️ 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".
| if exec_policy.get(field) == yolo_value: | ||
| del exec_policy[field] |
There was a problem hiding this comment.
In the openclaw --yolo --no-launch then openclaw --no-launch path, deleting the yolo tools.exec fields does not restore prompting: OpenClaw's gateway/node defaults for omitted exec policy are security=full and ask=off (docs), and the later approvals-file cleanup leaves approvals omitted so they fall back to that permissive policy. With no sandbox, host=auto resolves to gateway, so the regenerated plain recipe can still run exec without approval; reset to an explicit cautious/ask policy and matching approvals defaults instead of dropping these keys.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against openclaw 2026.6.11: with tools.exec omitted and the approvals file missing, exec-policy show reports the effective policy as security=full, ask=off, so deleting the yolo keys leaves auto-approval on. Fixed in a27d425: a non-yolo run now writes a prompting policy (security=allowlist, ask=on-miss), which prompts even after the approvals file is removed since the stricter of config and host approvals wins. The same applies to OpenCode, which defaults an unset permission to allow, so non-yolo now writes edit/bash/webfetch=ask instead of dropping the block.
There was a problem hiding this comment.
Correct, thank you. Verified against OpenClaw's own docs: an omitted tools.exec resolves to security=full, ask=off on the gateway host (sandboxing is off by default, so host=auto is the gateway), so deleting the keys keeps auto-approval on. Fixed in a27d425: a non-yolo run now writes an explicit prompting policy (security=allowlist, ask=on-miss) instead of deleting, replacing only a permissive/yolo policy and leaving a stricter hand-set one alone. The same permissive default applies to OpenCode (an unset permission defaults to allow), so its non-yolo path now writes edit/bash/webfetch=ask too. The file-edit CI drives these two agents with --yolo so the headless smoke test still runs tools.
…ive default OpenClaw and OpenCode both treat an omitted policy as permissive: OpenClaw's effective exec policy for an unset tools.exec is security=full/ask=off on the gateway host, and OpenCode defaults an unset permission to allow. So clearing the yolo values on a non-yolo run did not restore prompting, it fell back to those permissive defaults and left tool execution auto-approved. A non-yolo run now writes an explicit prompting policy: OpenClaw gets security=allowlist/ask=on-miss (verified to prompt even with the approvals file removed, since the stricter of config and approvals wins), and OpenCode gets edit/bash/webfetch=ask. Only a permissive/yolo value is tightened; a stricter deny (or an ask the user set) is preserved, and the yolo approvals defaults are still stripped. The file-edit CI path opts opencode/openclaw into --yolo, since those agents now prompt by default and the headless test needs auto-approval.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a27d425243
ℹ️ 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".
| permission = config.get("permission") | ||
| permission = permission if isinstance(permission, dict) else {} | ||
| to_ask = [t for t in ("edit", "bash", "webfetch") if permission.get(t, "allow") == "allow"] |
There was a problem hiding this comment.
When an existing OpenCode config uses a valid global permission rule such as "permission": "deny" or {"*": "deny"}, this reset path treats it as unset/default-allow and adds edit/bash/webfetch: "ask", replacing the string form entirely or overriding the catch-all. OpenCode's permissions docs allow both a single action value and * catch-all rules, so a non-yolo rerun can silently weaken stricter user-managed permissions instead of preserving them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against opencode 1.17.13 (a string permission normalizes to {"": value}, and a per-tool key overrides the catch-all). The reset now leaves a string permission in the config file as-is, treats a "" catch-all as the default an absent tool inherits, and only tightens an EFFECTIVE "allow" to "ask", so a "deny" string or {"*":"deny"} is no longer weakened (4681ac4).
| if exec_policy.get("security", "full") == "full" and exec_policy.get("ask", "off") == "off": | ||
| exec_policy = _subdict(_subdict(config, "tools"), "exec") | ||
| exec_policy.pop("host", None) # routing only; defaults to the gateway host | ||
| exec_policy["security"] = "allowlist" # only allowlisted commands skip approval | ||
| exec_policy["ask"] = "on-miss" # prompt on every non-allowlisted command |
There was a problem hiding this comment.
If the existing OpenClaw config uses the normalized tools.exec.mode knob (for example mode: "deny" or mode: "ask"), this branch treats the absent explicit security/ask fields as the permissive defaults and writes security plus ask alongside mode. OpenClaw's exec config documents that mode derives these fields and cannot be combined with explicit tools.exec.security/tools.exec.ask, so a non-yolo rerun can corrupt or relax an existing stricter policy rather than preserving it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, confirmed against the 2026.6.11 binary: with tools.exec.mode set, adding explicit security/ask makes OpenClaw reject the whole config ("tools.exec.mode cannot be combined with tools.exec.security or tools.exec.ask"), and it downgrades a stricter mode:deny/ask policy. The non-yolo reset now leaves any mode-based policy untouched (4681ac4).
| if to_ask: | ||
| permission = _subdict(config, "permission") | ||
| for tool in to_ask: | ||
| permission[tool] = "ask" |
There was a problem hiding this comment.
This only writes the new ask policy into the OPENCODE_CONFIG file, but OpenCode loads that custom config before project opencode.json and only OPENCODE_CONFIG_CONTENT has higher precedence. In a repo whose project config allows edit, bash, or webfetch, a plain non-yolo recipe still gets those project-level auto-approvals because the inline config for non-yolo only pins model; the reset needs to carry the ask policy at the same precedence as yolo does.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed with opencode debug config: with the ask policy only in OPENCODE_CONFIG (which loads below project opencode.json), a project config allowing edit/bash/webfetch still resolved to allow, so a non-yolo session auto-approved. write_opencode_config now returns the session permission and the callback carries it in OPENCODE_CONFIG_CONTENT (above project config) for non-yolo too, mirroring yolo, so a non-yolo session prompts. It never weakens a stricter user rule: the inline policy is deny when the user set deny, ask otherwise (4681ac4).
There was a problem hiding this comment.
Kept and reinforced in a5781bc. The non-yolo ask policy rides in OPENCODE_CONFIG_CONTENT (verified highest precedence, per-tool deep merge, higher-layer-wins), so a project config that allows edit/bash/webfetch still prompts. To reconcile with the related finding that this must not weaken a project deny, the inline value is now the stricter of "ask" and the value in our own session config: a deny or a pure deny/ask object we can see is carried inline verbatim, anything looser floors to "ask". A permissive project always prompts, and no path can produce an inline "allow" (silent auto-approve).
There was a problem hiding this comment.
Revisiting this after the deeper review: I have reverted the inline permission override for non-yolo runs in 249dbc6. Since OPENCODE_CONFIG_CONTENT outranks a project opencode.json that we cannot read, no value forced there can prompt over a permissive project without also weakening a stricter project rule (deny), a global string, a catch-all-less granular object, or a per-agent permission. The persistence bug is fully addressed by clearing our own config (flipping the explicit yolo allow back to ask); a project config is the user own deliberate choice and is now honored as written. --yolo still carries its allow inline so it works over a project config.
| if exec_policy.get("security", "full") == "full" and exec_policy.get("ask", "off") == "off": | ||
| exec_policy = _subdict(_subdict(config, "tools"), "exec") | ||
| exec_policy.pop("host", None) # routing only; defaults to the gateway host |
There was a problem hiding this comment.
When a user config has tools.exec.host: "sandbox" with no explicit security, OpenClaw's documented default is sandbox security=deny, not the gateway/node full default. This condition still interprets the missing field as full and then removes host, so a non-yolo rerun can broaden a sandbox-deny setup into gateway/auto exec with allowlist prompts instead of preserving the stricter sandbox policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed from exec.md (security default is deny for sandbox, full for gateway/node). --yolo only ever writes host=gateway, so a non-gateway host is user-set. The reset now only rewrites a gateway-routed permissive policy and never pops host when it is sandbox or node, so a sandbox-deny (or node) setup is preserved instead of being broadened to gateway/auto (4681ac4).
…rules on non-yolo reset
The non-yolo reset for openclaw/opencode assumed an omitted policy was the
permissive yolo default and rewrote it, which corrupted or weakened stricter
setups it should have preserved:
- OpenClaw tools.exec.mode is the normalized policy knob and cannot be combined
with explicit security/ask (OpenClaw rejects the whole config), so writing
security+ask alongside a mode:deny/ask policy both broke the config and
relaxed it. Leave a mode-based policy untouched.
- host=sandbox defaults to security=deny and host=node routes to a paired node;
neither is written by --yolo (which only writes host=gateway). Treating the
missing security as full and popping host broadened those into gateway/auto
exec. Only rewrite a gateway-routed permissive policy, and never pop a
non-gateway host.
- OpenCode permission can be a string ("deny") or a {"*": ...} catch-all.
The old code dropped a string form and overrode a catch-all by writing
per-tool ask, weakening a stricter user rule. Now a string is left in place,
a catch-all governs absent tools, and only an effective allow is tightened.
- The non-yolo ask policy only lived in OPENCODE_CONFIG, which loads below
project opencode.json, so a project config allowing edit/bash/webfetch still
auto-approved. Carry the ask policy in OPENCODE_CONFIG_CONTENT (above project
config) too, symmetric to how yolo carries its allow.
Also harden the openclaw path against a malformed non-dict tools value.
Adds tests for mode/sandbox/node hosts, string and catch-all permissions, and
the inline ask policy over a project config.
for more information, see https://pre-commit.ci
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d815b9160
ℹ️ 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".
| session_permission = { | ||
| t: "deny" if permission.get(t, default) == "deny" else "ask" for t in tools | ||
| } |
There was a problem hiding this comment.
When the persisted OpenCode config uses the granular object syntax, e.g. permission: {"bash": {"*": "deny"}}, this expression sees a dict and emits "bash": "ask" in OPENCODE_CONFIG_CONTENT. That env config outranks the session/project files, so a user’s deny policy is weakened to an approvable action for non-yolo runs; object-valued rules need to preserve deny/catch-all semantics instead of being collapsed to ask.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Right, confirmed with the 1.17.13 binary: an object value like {"bash":{"*":"deny"}} was being collapsed to inline "bash":"ask", and since OPENCODE_CONFIG_CONTENT deep-merges over project config that weakened the deny. Fixed in a5781bc: a granular object that grants nothing (ask/deny only) now rides inline verbatim and is preserved in the file. Note an object that grants "allow" anywhere is floored to the string "ask" instead (a string fully replaces a project object, so no inline allow pattern can leak into a silent auto-approve).
| permissive = ( | ||
| "mode" not in exec_policy | ||
| and host in (None, "gateway", "auto") | ||
| and exec_policy.get("security", "full") == "full" | ||
| and exec_policy.get("ask", "off") == "off" |
There was a problem hiding this comment.
When OpenClaw is using its implicit/auto sandbox host, the omitted exec policy is not the gateway's permissive default; the surrounding comment already notes sandbox defaults to security=deny. Treating host in (None, "auto") as yolo-permissive rewrites a fresh non-yolo config to allowlist/on-miss, so users with an active sandbox go from deny-by-default to commands being allowed after a prompt even though there was no stale yolo state to clear.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Confirmed against exec.md: with sandbox off, host=auto resolves to gateway (security=full), but with an active sandbox an omitted/auto host defaults to security=deny, so the old predicate would broaden a fresh sandboxed config from deny to allowlist. --yolo only ever writes host=gateway/security=full/ask=off explicitly, so a5781bc scopes the reset to exactly that fingerprint and leaves None/auto/sandbox/node untouched.
| if session_permission: | ||
| inline_config["permission"] = session_permission |
There was a problem hiding this comment.
For non-yolo runs with the usual generated session policy, this writes {"bash":"ask","edit":"ask","webfetch":"ask"} into OPENCODE_CONFIG_CONTENT, which the adjacent comment says outranks the repo's own opencode.json. In a project that deliberately has permission: {"bash":"deny"} (or denies edits/webfetch), the inline block weakens that deny to an approvable action; the override should only defeat permissive project rules, not stricter ones.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Verified the merge with the 1.17.13 binary: OPENCODE_CONFIG_CONTENT is per-tool deep-merged and higher-layer-wins, not stricter-wins, so an inline "ask" does override a project "bash":"deny". Since we cannot read the project opencode.json, no single inline value both prompts over a permissive project (comment 3524985232) and preserves an unobservable project deny.
Landed one coherent design in a5781bc: the inline value per tool is the stricter of "ask" and the value in our OWN session config. A deny or a pure deny/ask object a user placed in the reused session dir rides inline verbatim, so a stricter rule we can see is never weakened; anything looser (allow, or an object granting allow) floors to "ask". The one residual case, a deny that exists only in the project config, is softened to "ask", a prompt, never a silent auto-approve, which keeps the non-yolo contract intact. Escalating to inline "deny" instead would hard-block tools in a plain repo, a worse default for an interactive session.
| permissive = ( | ||
| "mode" not in exec_policy | ||
| and host in (None, "gateway", "auto") |
There was a problem hiding this comment.
OpenClaw's docs define tools.exec.mode: "full" as running host exec without approval prompts, but this predicate skips every mode-based policy, so a config that has been normalized/migrated from the yolo security=full, ask=off form remains in no-approval mode after unsloth start openclaw --no-launch without --yolo. The reset needs to treat mode: "full" as stale yolo state rather than preserving it as a stricter user policy.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Checked against the 2026.6.11 binary: OpenClaw never migrates our security=full/ask=off write into mode:"full". Feeding that exact config through config validate, config get tools.exec, and doctor --fix leaves security/ask in place and never introduces a mode key, and mode cannot even coexist with security/ask (the config is rejected). So a mode:"full" is always a deliberate user policy, not stale yolo state, and clobbering it would override a genuine user intent. Left as-is.
…ular denies OpenClaw: reset only the exact host=gateway/security=full/ask=off policy --yolo writes, so an omitted or host=auto/sandbox/node policy (which can resolve to a sandbox security=deny default) is no longer broadened to allowlist/on-miss, and a deliberate tools.exec.mode is left alone (OpenClaw never migrates our security/ask write into a mode). OpenCode: carry a granular object or a deny inline verbatim so a per-tool user rule is not collapsed to a blanket ask, but floor any object that grants allow anywhere to the string ask (which fully replaces a project object) so no inline allow pattern can leak through into a silent auto-approve on a non-yolo session.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5781bc832
ℹ️ 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".
| # through. A plain "allow" or an absent tool also floors to "ask". | ||
| if isinstance(value, dict): | ||
| # Copy so the returned inline dict never aliases config["permission"]. | ||
| return copy.deepcopy(value) if not _has_allow(value) else "ask" |
There was a problem hiding this comment.
When the stored OpenCode permission is a granular object with only ask/deny entries, this branch copies it into OPENCODE_CONFIG_CONTENT, which is loaded after project config. Per the OpenCode permissions docs, granular rules are just pattern matches and unspecified permissions fall back to permissive defaults, so a session config like {"bash":{"git *":"ask"}} will override a project's bash: "ask" and still auto-approve non-matching commands such as npm test in a non-yolo run. Collapse these objects to "ask" or add a "*": "ask" floor before inlining them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 249dbc6: a non-yolo session no longer inlines any permission, so a granular object with no catch-all is left in our config file as the user wrote it and cannot override a project rule or auto-approve unmatched commands.
| if defaults.get(field) == yolo_value: | ||
| del defaults[field] |
There was a problem hiding this comment.
This removes any default field whose value happens to match the YOLO payload, even when the approvals file is a user-managed mixed policy rather than stale YOLO state. For example defaults={"security":"allowlist","ask":"on-miss","askFallback":"full"} loses askFallback; OpenClaw's docs state an omitted askFallback defaults to deny, so a plain non-yolo rerun changes headless/no-UI fallback behavior despite not matching the YOLO tuple. Only strip defaults when the full YOLO fingerprint is present.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 249dbc6: the approvals cleanup now strips the defaults only when the full yolo fingerprint (security=full, ask=off, askFallback=full) is present, so a mixed policy that merely shares askFallback=full is kept intact.
| # A string permission ("deny"/"ask"/"allow") is a global user rule left in the | ||
| # config file as-is; "deny" rides inline verbatim, anything looser floors to | ||
| # "ask". | ||
| session_permission = {t: permission if permission == "deny" else "ask" for t in tools} |
There was a problem hiding this comment.
When the session config contains a global string policy such as "permission": "deny" or "ask", this collapses that catch-all into only the three yolo-related tools before putting it in OPENCODE_CONFIG_CONTENT. Because the inline config has higher precedence than OPENCODE_CONFIG, the object replaces the original string policy and other permissions fall back to OpenCode's defaults (mostly allow), so a user-managed global deny/ask is weakened on non-yolo runs. Carry the string through unchanged, or inline an equivalent "*" catch-all.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 249dbc6 by removing the non-yolo inline override entirely. A global string permission is now left in our config file untouched and nothing is forced into OPENCODE_CONFIG_CONTENT, so it can no longer be narrowed to the three tools.
| if yolo: | ||
| inline_config["permission"] = {"edit": "allow", "bash": "allow", "webfetch": "allow"} | ||
| if session_permission: | ||
| inline_config["permission"] = session_permission |
There was a problem hiding this comment.
This only inlines a top-level permission, but OpenCode also supports per-agent permissions and documents that agent rules are merged with global config and take precedence. In a repo with agent.build.permission allowing bash/edit/webfetch (the default agent is build), a non-yolo unsloth start opencode still auto-approves those tools despite this inline ask policy. Inline or neutralize the active/default agent permission as well.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 249dbc6: since a non-yolo session no longer forces a top-level permission over the project config, per-agent permissions are honored as written rather than partially overridden.
| # through. A plain "allow" or an absent tool also floors to "ask". | ||
| if isinstance(value, dict): | ||
| # Copy so the returned inline dict never aliases config["permission"]. | ||
| return copy.deepcopy(value) if not _has_allow(value) else "ask" |
There was a problem hiding this comment.
For any granular permission object that contains an allow, this collapses the whole tool policy to the string "ask". A user/session rule like {"bash":{"rm *":"deny","git *":"allow"}} therefore becomes approvable for rm * in the higher-precedence inline config on a non-yolo run, weakening an explicit deny while trying to remove stale allows. Preserve the object and rewrite only its allow entries to ask, with a safe catch-all, instead of replacing the whole rule.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed in 249dbc6: with the inline override removed, a granular object (including one mixing deny and allow) is left in our config verbatim rather than collapsed, so an explicit deny is never weakened.
…ngerprint The non-yolo OpenCode reset carried a session permission in OPENCODE_CONFIG_CONTENT, which outranks the project opencode.json we cannot read. That inline override could not correctly reflect the project: it weakened a project deny to a prompt, mishandled global string rules, leaked through a granular object's permissive default when no catch-all was present, collapsed an object with an allow (losing its deny), and missed per-agent permissions. All of these stem from forcing a value over an unknown project config. A non-yolo run now only undoes what --yolo wrote: it flips our own explicit per-tool allow back to ask in our config file and carries no permission inline, so the project's own permissions are honored as written. Clearing our persisted yolo state is the actual fix; --yolo still carries its allow inline so it works over a project config. OpenClaw approvals cleanup now strips the yolo defaults only when the full fingerprint (security=full, ask=off, askFallback=full) is present, so a mixed user policy that merely shares askFallback=full (whose omitted default is deny) is kept intact.
for more information, see https://pre-commit.ci
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. 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". |
1 participant
Problem
unsloth start <agent> --no-launchreuses a stable Unsloth-owned config directory across runs (by design, since a previously printed recipe may still be running an agent whose sessions live there). The--yolopath writes auto-approval settings into those persistent files, but runs without--yolonever removed them:tools.exec = {host: gateway, security: full, ask: off}inopenclaw.json, plusexec-approvals.jsonwithsecurity=full/ask=offdefaults.permission = {edit: allow, bash: allow, webfetch: allow}inopencode.json.So after a single
--yolo --no-launchrun, every later--no-launchrun of the same agent without--yolostill had tool execution silently pre-approved. That defeats the confirmation gate the non-yolo mode is supposed to provide: any prompt-injected or otherwise malicious instruction the agent follows can run commands or edit files without the user ever being asked.Repro on main:
The other agents (claude, codex, hermes, pi) are unaffected: their yolo form is a command line flag, nothing is persisted. Launch mode is unaffected: it uses an ephemeral temp dir that is deleted when the agent exits.
Fix
Runs without
--yolonow reset the persisted state instead of leaving it behind:host/security/askkeys the yolo path writes are stripped fromtools.exec(empty parents removed), and the yolodefaultsare stripped fromexec-approvals.json. Approvals OpenClaw itself recorded in that file are preserved; the file is deleted when only the yolo payload is left. The CLI echoesRemoved .../exec-approvals.jsonso the reset is visible.permissionblock is dropped.Any other user or runtime state in the session dir is left alone, so the never-wipe contract of the no-launch dir still holds.
Validation
--yolo --no-launchthen--no-launchon the same dir leaves no auto-approval state, session provider intact) and at the writer level, including one that checks OpenClaw runtime-recorded approvals survive the reset and one for foreigntools.execkeys. Full suite: 138 passed.unsloth start openclaw/opencode --no-launch --yolofollowed by the same command without--yoloon a fresh Studio home. Before the fix the exec policy, approvals file, and permission block all persist; after the fix the rerun removes all three and the printed recipe still works.