Skip to content

fix(ui): read tools.exec.security in Quick Settings instead of agents.defaults path#78340

Closed
XuZehan-iCenter wants to merge 3 commits into
openclaw:mainfrom
XuZehan-iCenter:fix/ui-exec-security-config-path
Closed

fix(ui): read tools.exec.security in Quick Settings instead of agents.defaults path#78340
XuZehan-iCenter wants to merge 3 commits into
openclaw:mainfrom
XuZehan-iCenter:fix/ui-exec-security-config-path

Conversation

@XuZehan-iCenter
Copy link
Copy Markdown
Contributor

Fixes #78311

The Control UI Quick Settings security badge was reading cfg.agents.defaults.exec.security, which does not exist in current configs. The exec security setting has lived under tools.exec.security since v2026.4.x.

Real behavior proof

Before fix:

  • User sets tools.exec.security to "blocklist" in config
  • Quick Settings badge still shows allowlist (wrong path)

After fix:

  • Quick Settings badge correctly reads tools.exec.security and shows blocklist
  • Falls back to agents.defaults.exec.security only if tools.exec.security is absent (backward-compatible)

Changelog

Formatting

pnpm exec oxfmt --check --threads=1 ui/src/ui/app-render.ts — passed.

Closes #78311

@openclaw-barnacle openclaw-barnacle Bot added app: web-ui App: web-ui size: XS triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 6, 2026
@clawsweeper
Copy link
Copy Markdown
Contributor

clawsweeper Bot commented May 6, 2026

Thanks for the context here. I swept through the related work, and this is now duplicate or superseded.

Close as superseded: this PR addresses a real current-main Control UI bug, but the same remaining fix is already covered by the more complete open implementation at #78448, which includes the changelog entry and sufficient real behavior proof this PR still lacks.

So I’m closing this here and keeping the remaining discussion on the canonical linked item.

Review details

Best possible solution:

Land a single canonical fix that reads the documented tools.exec.security path, ignores the non-schema legacy path, includes regression coverage and a changelog entry, then close the duplicate PRs against that canonical implementation.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection reproduces the current-main bug: render Quick Settings with configForm or configSnapshot.config containing tools.exec.security and no agents.defaults.exec.security, and the helper still returns the default allowlist badge value.

Is this the best way to solve the issue?

No for this PR as the merge path. The patch direction is now reasonable, but #78448 is the safer canonical solution because it tracks the same bug with changelog coverage and sufficient real behavior proof.

Security review:

Security review cleared: The diff is limited to Control UI config display logic and a jsdom unit test; it does not add dependencies, workflows, secret handling, or code-execution paths.

What I checked:

Likely related people:

  • BunsDev: Recent GitHub path history shows repeated work on ui/src/ui/app-render.ts and Quick Settings, including responsive Control UI and Quick Settings layout changes adjacent to this helper and badge surface. (role: recent Control UI maintainer; confidence: high; commits: 60171e863882, 098b72910dea, 5e8cb77e7917; files: ui/src/ui/app-render.ts, ui/src/ui/views/config-quick.ts)
  • steipete: Recent exec-infra history touches src/infra/exec-approvals-effective.ts, which defines the tools.exec policy scope that the UI should mirror. (role: exec policy contract maintainer; confidence: medium; commits: 45dee50c2860, df525b90f29f; files: src/infra/exec-approvals-effective.ts, docs/tools/exec.md)
  • Takhoffman: The local exec-policy CLI work established the user-facing tools.exec.security editing path that this Quick Settings badge reports. (role: adjacent exec-policy CLI owner; confidence: medium; commits: 4bf94aa0d669; files: src/cli/exec-policy-cli.ts, docs/cli/approvals.md, docs/tools/exec.md)

Codex review notes: model gpt-5.5, reasoning high; reviewed against 1f822d7c222c.

@XuZehan-iCenter XuZehan-iCenter force-pushed the fix/ui-exec-security-config-path branch 2 times, most recently from 3653ce9 to aa9b36e Compare May 6, 2026 07:35
@XuZehan-iCenter XuZehan-iCenter force-pushed the fix/ui-exec-security-config-path branch from aa9b36e to 4b09c27 Compare May 6, 2026 07:36
@openclaw-barnacle openclaw-barnacle Bot removed the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 6, 2026
….defaults path

The Control UI Quick Settings security badge was reading
cfg.agents.defaults.exec.security, which does not exist in current
configs. The exec security setting has lived under 	ools.exec.security
since v2026.4.x. This restores the correct exec-policy display
(�llowlist, �locklist, etc.) in the UI.

Backward-compatible: falls back to the old �gents.defaults.exec.security
path if 	ools.exec.security is absent, preserving behavior for legacy
configs.

Closes openclaw#78311
@openclaw-barnacle openclaw-barnacle Bot added the triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. label May 6, 2026
@byungskers

This comment was marked as low quality.

@XuZehan-iCenter
Copy link
Copy Markdown
Contributor Author

Clean migration path! Reading from "tools.exec.security" first while keeping the old "agents.defaults.exec.security" as fallback is the right approach for backward compatibility. The code is easy to follow. One minor suggestion: it might be worth adding a small inline comment or deprecation note so future maintainers know when the old path can be safely removed. Otherwise LGTM! ✅

Thanks for the review! I had originally planned to add a changelog entry, but since the branch merge timing was sensitive and the change was relatively small, I skipped it this time to avoid conflicts. I’ll make sure to include changelog updates and inline notes in future PRs to keep the style consistent. Appreciate the suggestion!

- Remove legacy agents.defaults.exec.security fallback (schema does not define this path)
- Use path-presence check instead of sentinel-value check for tools.exec.security
- Trim whitespace on security value, consistent with gatewayAuth normalization
- Export extractQuickSettingsSecurity for regression test coverage
- Add 6-case jsdom regression test covering canonical path, legacy-path ignore,
  missing-config fallback, whitespace trim, empty-string fallback, and absent-config sentinel

Fixes openclaw#78311
@clawsweeper clawsweeper Bot closed this May 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app: web-ui App: web-ui size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: Control UI Settings page shows wrong Exec Policy — reads from wrong config path

2 participants