fix(cli): show allowlist grant scope in approvals get by ylcn91 · Pull Request #138386 · openclaw/openclaw

ylcn91 · 2026-09-04T16:07:39Z

What Problem This Solves

openclaw approvals get displayed only the executable pattern and last-use time. A manual rule allowing any arguments, a directory-bound generated grant, and an inactive legacy grant therefore looked identical during a permissions audit.

Fixes #137452.

Why This Change Was Made

Classify each grant beside the executable matcher and render that classification in a Scope column. Doctor uses the same classifier while retaining its generated-source ownership guard, so inactive manual rules remain untouched.

This preserves the contributor's repair and both earlier classification corrections. Matching, grant creation, stored fields, JSON output, prompts, and Doctor deletion behavior remain unchanged.

User Impact

Scope	Matching constraint
`any args`	Manual executable rule without an argument restriction
`argv`	Manual argument regular expression
`argv+cwd`	Exact arguments and working directory
`command text`	Generated command or node-command marker
`inactive`	Legacy executable grant that cannot match

The scope describes the rule's matching constraint; effective approval policy still applies. Narrow terminals wrap the executable pattern while retaining the scope distinction. Reserved command markers without generated source retain their ordinary manual-rule classification. Doctor does not delete manual legacy hashes.

Evidence

Real compiled CLI baseline: nine same-pattern grants rendered identically despite different evaluator outcomes.
Candidate 72c5ecef576ff6964b721d48c22a9244d4ae84cc: 13 synthetic grants at terminal widths 120, 80, 60, and 40; full JSON snapshots remained identical to baseline.
Published evaluator: changed arguments, changed directory, joined-versus-separate arguments, generated/manual markers, real reserved-marker executable basenames, and broad-rule fallback retained their baseline outcomes.
Real doctor --fix --non-interactive: baseline and candidate removed the same three obsolete generated grants and retained the same ten entries, including manual legacy hashes and generated command markers.
Hosted CI run 34055969719: approval CLI tests (including both narrow-width regressions), matcher, durable-grant, generated-migration, Doctor-health, lint, production/test type checks, and dependency checks passed. The broader staging run is not claimed fully green; final required checks remain the merge gate.
Fresh independent source review found no actionable findings.

Earlier contributor corrections remain covered: generated source is required for command-text scope; source-less legacy hashes are inactive but remain outside Doctor deletion ownership.

Independent source-blind acceptance passed the observable contract, including 74 evaluator probes, reordered/empty lists, privacy canaries, 108 paired approval-policy combinations, real grant creation/repetition, and paired Doctor cleanup. Prompt requirements were tested through the published API; interactive prompt screens were not exercised.

clawsweeper · 2026-09-04T16:07:43Z

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

clawsweeper · 2026-09-04T17:44:37Z

Codex review: needs maintainer review before merge. Reviewed September 6, 2026, 4:11 PM ET / 20:11 UTC.

ClawSweeper review

What this changes

Adds grant-scope labels to the approvals CLI table and shares their classification with Doctor while preserving existing cleanup behavior.

Merge readiness

✅ Ready for maintainer review

This PR remains useful: current main and the latest release still omit grant scope from the allowlist table. Both earlier classification findings are resolved, and no new blocking defect was found.

Priority: P2
Reviewed head: 72c5ecef576ff6964b721d48c22a9244d4ae84cc

Review scores

Measure	Result	What it means
Overall readiness	🐚 platinum hermit (4/6)	A focused repair with relevant production observations, meaningful regression coverage, and no remaining actionable finding.
Proof confidence	🦞 diamond lobster (5/6)	Sufficient (live_output): The captured body records compiled CLI before/after observations for the reviewed candidate, including distinct grant scopes across four terminal widths, unchanged JSON and evaluator results, and equivalent real Doctor cleanup. These scenarios cover the changed production owners; local tests were not run by this reviewer.
Patch quality	🐚 platinum hermit (4/6)	No actionable review findings were identified.

Verification

Check	Result	Evidence
Real behavior	Verified	Sufficient (live_output): The captured body records compiled CLI before/after observations for the reviewed candidate, including distinct grant scopes across four terminal widths, unchanged JSON and evaluator results, and equivalent real Doctor cleanup. These scenarios cover the changed production owners; local tests were not run by this reviewer.
Evidence reviewed	10 items	Current main still needs the display repair: The fetched main revision builds allowlist rows from target, agent, pattern, and last-use time, without displaying source or argument restrictions. Latest release also lacks allowlist scope: The supplied latest release, v2026.9.2, retains the four-column allowlist table. Its existing Scope column belongs to the separate effective-policy table. Classification and prior findings: The classifier requires generated source for command-text markers and labels legacy argument hashes inactive regardless of source. These resolve the two concrete findings retained in the previous review history. The executable matcher remains unchanged.
Findings	None	None.
Security	None	None.

How this fits together

The approvals CLI reads local or remote execution rules and presents them to operators. The new scope labels explain each rule’s matching constraint; Doctor also uses the classifier to identify obsolete generated grants.

flowchart TD
  A[Stored approval rules] --> B[Approvals CLI]
  B --> C[Grant scope classifier]
  C --> D[Human-readable allowlist table]
  A --> E[Doctor cleanup]
  C --> E
  E --> F[Remove obsolete generated grants]
  B --> G[Unchanged JSON snapshot]

Before merge

None.

Agent review details

Security

None.

PR surface

Source +26, Tests +116. Total +142 across 5 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	3	36	10	+26
Tests	2	116	0	+116
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	5	152	10	+142

Review metrics

None.

Root-cause cluster

Relationship: fixed_by_candidate
Canonical: #137452
Summary: This PR directly addresses the canonical allowlist-display issue; the linked CI repair is distinct.

Members:

canonical: openclaw approvals get drops source and argPattern from the Allowlist table, so an argv- and cwd-bound allow-always grant renders identically to a broad path grant #137452 - Reports the exact omission of grant constraints from the human-readable allowlist table.
adjacent_distinct: test(agents): give the sibling-drain completion wait a tolerance on slow runners #139176 - Merged repair for an unrelated CI timing failure encountered during validation.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Technical review

Best possible solution:

Present matching constraints beside each allowlist rule while retaining existing authorization, JSON output, and Doctor ownership semantics.

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

Yes: current main renders identical-pattern grants without their source or argument constraints, so otherwise identical broad, restricted, and inactive entries are indistinguishable. This was established from source; no local execution was performed.

Is this the best way to solve the issue?

Yes: the shared classifier repairs the human-readable view using existing matching semantics, and its generated-source guard preserves Doctor’s cleanup boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 800ee0c0ed0a.

Labels

Label justifications:

P2: Repairs misleading permission-audit output without changing execution policy or disrupting a core workflow.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The captured body records compiled CLI before/after observations for the reviewed candidate, including distinct grant scopes across four terminal widths, unchanged JSON and evaluator results, and equivalent real Doctor cleanup. These scenarios cover the changed production owners; local tests were not run by this reviewer.
proof: sufficient: Contributor real behavior proof is sufficient. The captured body records compiled CLI before/after observations for the reviewed candidate, including distinct grant scopes across four terminal widths, unchanged JSON and evaluator results, and equivalent real Doctor cleanup. These scenarios cover the changed production owners; local tests were not run by this reviewer.

Evidence

What I checked:

Current main still needs the display repair: The fetched main revision builds allowlist rows from target, agent, pattern, and last-use time, without displaying source or argument restrictions. (src/cli/exec-approvals-cli.ts:951, 800ee0c0ed0a)
Latest release also lacks allowlist scope: The supplied latest release, v2026.9.2, retains the four-column allowlist table. Its existing Scope column belongs to the separate effective-policy table. (src/cli/exec-approvals-cli.ts:951, 3928bad9badf)
Classification and prior findings: The classifier requires generated source for command-text markers and labels legacy argument hashes inactive regardless of source. These resolve the two concrete findings retained in the previous review history. The executable matcher remains unchanged. (src/infra/exec-command-resolution.ts:280, 72c5ecef576f)
Doctor preserves deletion ownership: Comparing the introduced predicate with its pinned base shows equivalent cleanup: only generated, non-command-marker grants without cwd-bound hashes are removed. Manual legacy hashes remain outside Doctor deletion ownership. (src/infra/exec-approvals-generated-migration.ts:7, 72c5ecef576f)
Existing matching contract: Documentation distinguishes manual argument regexes, directory-bound generated grants, inactive older generated grants, and manual rules retained by Doctor. The CLI reference also explains that effective policy still governs execution. Public docs: docs/tools/exec-approvals.md. (docs/tools/exec-approvals.md:411, 72c5ecef576f)
After-fix production behavior evidence: The supplied PR body at captured sourceRevision 40a1f787e44d8b8446425436b4b570706976adc1e0561834c63091f96b9f492b reports compiled baseline/candidate CLI comparisons for 13 grants at widths 120, 80, 60, and 40, identical JSON snapshots, unchanged evaluator outcomes, and real Doctor runs removing the same three generated entries while retaining ten. This directly exercises the changed rendering and cleanup owners. The recorded candidate is the reviewed head; these are supplied observations, not commands executed by this reviewer. (72c5ecef576f)

Likely related people:

Josh Avant: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
Peter Steinberger: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
obviyus: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (7 earlier review cycles)

reviewed 2026-09-04T17:43:01.403Z sha a42f764ca4d45671105f13383fe169f69b995a83 :: needs real behavior proof before merge. :: [P2] Require generated source for command-text scope
reviewed 2026-09-04T19:44:17.004Z sha a4e3ce15daea50527e32ff755bcd7d68beeb180b :: blocked before merge. :: none
reviewed 2026-09-04T20:54:05.065Z sha a4e3ce15daea50527e32ff755bcd7d68beeb180b :: blocked before merge. :: none
reviewed 2026-09-04T21:49:50.523Z sha 23d6eda72469abb199c38030c795e0619779e62f :: blocked before merge. :: none
reviewed 2026-09-05T14:38:01.919Z sha 266794c67cd32e12bdeca19dd8ac893b5744e31f :: needs changes before merge. :: [P2] [P2] Mark source-less legacy argv hashes as inactive
reviewed 2026-09-05T16:11:47.947Z sha 200a5a393b8e2d2100f996706a98864e64bac66a :: blocked before merge. :: none
reviewed 2026-09-05T18:40:25.800Z sha b3a14f4 :: needs maintainer review before merge. :: none

ylcn91 · 2026-09-04T19:36:16Z

Applied the P2 in a4e3ce1: the command text scope is now gated on source: "allow-always", so a manual =command:… pattern added through approvals allowlist add renders its real (any args) scope; ordering is unchanged, so generated grants still classify before the inactive check and doctor --fix keeps skipping them. Regression case added (fails on a42f764). Real openclaw approvals get before/after against an isolated state dir with a path-only entry, a manual =command: pattern, and a generated grant is in the PR body under "ClawSweeper review of a42f764".

ylcn91 · 2026-09-05T14:12:24Z

Dropped the docs/cli/approvals.md hunk in 2c1b15e4080 so the PR no longer touches a security-owned CODEOWNERS path; the Scope column semantics are now described in the PR body under User Impact. Code and tests are unchanged from 23d6eda (src/cli/exec-approvals-cli.test.ts: 43 passed on the new head).

@clawsweeper re-review

clawsweeper · 2026-09-05T14:12:29Z

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event exact_review_queue).
Result: when the review finishes, ClawSweeper will create the durable review comment if needed or update the existing comment in place.

`renderApprovalsSnapshot` built each Allowlist row from `pattern` and `lastUsedAt` only, dropping `source` and `argPattern` — the two fields `matchAllowlist` branches on. A cwd- and argv-bound `allow-always` grant, an `argPattern`-restricted rule, and an inactive legacy generated grant therefore rendered byte-identically to a broad path grant, so the CLI answered "what can this agent run" wrong in the permissive direction on the surface operators use for permissions audits. Record the scope fact at the authorization owner: `classifyExecAllowlistScope` in exec-command-resolution.ts derives what an entry actually authorizes from the same fields the matcher reads, and the Allowlist table renders it as a `Scope` column. `exec-approvals-generated-migration.ts` drops its duplicate inactive-grant predicate and reads the same classifier, so doctor and the audit table can no longer disagree about which grants are dead. Fixes openclaw#137452

`classifyExecAllowlistScope` labeled any pattern with a `=command:` or `=node-command:` prefix as `command text`, but only a grant carrying `source: "allow-always"` is ever honored as one: `hasExactCommandDurableExecApproval` and `hasNodeCommandAllowAlwaysMarker` both require that source, and `matchAllowlist` has no reserved-prefix branch at all. `approvals allowlist add` stores any nonempty pattern with no source, so a manual reserved-prefix entry was rendered as a narrow exact-command grant while the matcher treats it as an ordinary path-only glob. The audit table therefore understated its scope — the same collapse this branch fixes, in the other direction. Gate the branch on the source; a source-less pattern falls through to its real scope. Ordering is unchanged, so a generated exact-command grant is still classified before the `inactive` check and `doctor --fix` keeps skipping it.

…owned reference docs/cli/approvals.md is a CODEOWNERS path with security ownership, so the paragraph describing the Allowlist table's Scope column pulled a security-owner review gate onto a CLI output fix. Restore the page to its main version; the Scope column values and their meaning are described in the PR body instead. No code change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

…ants matchArgPattern rejects every legacy sha256:argv hash whatever its source, so a hand-written entry carrying one authorizes nothing. The Scope column showed it as a live "argv" restriction; it now reads "inactive" like the generated form. Doctor keeps its allow-always guard, so the newly inactive manual entries stay in place for the operator instead of being deleted. Regression: approvals get renders a manual legacy hash as inactive, and the Doctor migration retains a manual legacy hash while removing generated ones.

Preserve contributor work, share grant classification with Doctor, and verify scope distinctions in narrow terminals without changing matching or JSON output.

obviyus · 2026-09-06T20:16:21Z

Thanks @ylcn91 — landed in bc5e0f0933b2e821b3310e849766a2588eb7c628, preserving your scope repair and both classification corrections.

Bug: grants with the same executable pattern looked identical despite different matching authority.
Fix: the CLI shows a shared scope classification, and Doctor retains its generated-grant deletion boundary.
Proof: compiled CLI with 13 grants at four terminal widths; unchanged JSON and evaluator outcomes; paired Doctor cleanup; 108 paired prompt-policy cases and real grant creation/repetition; independent acceptance and source review.
CI: exact-head run 34056883440 passed the required gate. No CI exception was used.

Before	After
Same-pattern broad, restricted, and inactive rules looked identical	`any args`, `argv`, `argv+cwd`, `command text`, and `inactive` identify their matching scope

Prompt requirements were checked through the published API; interactive prompt screens were not exercised. Matching, stored fields, JSON output, and Doctor deletion semantics remain unchanged.

…rol-ui * origin/main: docs: remove the empty release-sections nav group (#140427) fix(tooling): omit Codex machine squash credit (#140420) chore(ui): refresh control ui locales (#140384) refactor(plugin-sdk): reuse native approval input contracts (#140415) fix: silent model WebSocket streams hang past the idle timeout (#140335) perf(ai): avoid unused text extraction for image results (#140409) fix(ui): distinguish failed GitHub status checks from disconnected accounts (#140333) refactor(cron): share runtime feedback schema fields (#140408) fix: avoid duplicate cloud worker teardown during recovery (#140377) fix(cli): show executable approval scope (#138386) refactor(ui): reuse shared requirement records (#140407) test(gateway): table-drive device token lifecycle checks (#140403) fix(chat): fence delayed history at terminal retirement (#140379) test(skills): compare large upload bytes natively (#140402) refactor(delivery): reuse the canonical rendered batch plan type (#140386) docs(start): repair the first-run path from landing page to first channel (#140390) # Conflicts: # docs/.i18n/glossary.zh-CN.json

Show each executable grant's matching scope in approvals get and reuse the classification in Doctor without changing authorization, stored fields, or JSON output. Preserve the contributor's reserved-marker and legacy-hash corrections. Cover narrow-terminal scope distinctions and verify the compiled CLI, evaluator, approval policy, grant persistence, and Doctor cleanup. Fixes openclaw#137452. Co-authored-by: Ayaan Zaidi <hi@obviy.us>

ylcn91 marked this pull request as ready for review September 4, 2026 16:07

ylcn91 requested a review from a team as a code owner September 4, 2026 16:07

openclaw-barnacle Bot added docs Improvements or additions to documentation cli CLI command changes size: S labels Sep 4, 2026

ylcn91 force-pushed the fix/approvals-get-allowlist-scope branch from a4e3ce1 to 23d6eda Compare September 4, 2026 21:17

clawsweeper Bot added the merge-risk: 🚨 security-boundary 🚨 May affect sandboxing, authorization, credentials, or sensitive data. label Sep 4, 2026

openclaw-barnacle Bot removed the docs Improvements or additions to documentation label Sep 5, 2026

ylcn91 force-pushed the fix/approvals-get-allowlist-scope branch from 2c1b15e to 266794c Compare September 5, 2026 14:31

ylcn91 mentioned this pull request Sep 5, 2026

test(agents): give the sibling-drain completion wait a tolerance on slow runners #139176

Merged

ylcn91 force-pushed the fix/approvals-get-allowlist-scope branch from 266794c to 200a5a3 Compare September 5, 2026 16:04

openclaw-barnacle Bot added the agents Agent runtime and tooling label Sep 5, 2026

ylcn91 and others added 4 commits September 5, 2026 18:34

ylcn91 force-pushed the fix/approvals-get-allowlist-scope branch from 200a5a3 to b3a14f4 Compare September 5, 2026 18:34

obviyus self-assigned this Sep 6, 2026

fix(cli): distinguish executable approval scopes

72c5ece

Preserve contributor work, share grant classification with Doctor, and verify scope distinctions in narrow terminals without changing matching or JSON output.

obviyus merged commit bc5e0f0 into openclaw:main Sep 6, 2026
210 of 212 checks passed

github-actions Bot mentioned this pull request Sep 6, 2026

📡 Upstream Digest — 2026-09-06 21:52 UTC curtismercier/openclaw-mods#1801

Open

clawsweeper Bot mentioned this pull request Sep 23, 2026

fix(cli): include derived Scope column in approvals get allowlist table (#137452) #137878

Open

Navigation Menu

Repository navigation

fix(cli): show allowlist grant scope in approvals get - #138386

fix(cli): show allowlist grant scope in approvals get#138386
obviyus merged 5 commits into
openclaw:mainfrom
ylcn91:fix/approvals-get-allowlist-scope

ylcn91 commented Sep 4, 2026 •

edited by obviyus

Loading

Uh oh!

clawsweeper Bot commented Sep 4, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Sep 4, 2026 •

edited

Loading

Security

PR surface

Review metrics

Root-cause cluster

Technical review

Labels

Evidence

Rating scale

Workflow

History

Uh oh!

ylcn91 commented Sep 4, 2026

Uh oh!

ylcn91 commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading

Uh oh!

Uh oh!

obviyus commented Sep 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

ylcn91 commented Sep 4, 2026 • edited by obviyus Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review complete

Uh oh!

clawsweeper Bot commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

ClawSweeper review

What this changes

Merge readiness

Review scores

Verification

How this fits together

Before merge

Security

PR surface

Review metrics

Root-cause cluster

Technical review

Labels

Evidence

Rating scale

Workflow

History

Uh oh!

ylcn91 commented Sep 4, 2026

Uh oh!

ylcn91 commented Sep 5, 2026

Uh oh!

clawsweeper Bot commented Sep 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

obviyus commented Sep 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ylcn91 commented Sep 4, 2026 •

edited by obviyus

Loading

clawsweeper Bot commented Sep 4, 2026 •

edited

Loading

clawsweeper Bot commented Sep 4, 2026 •

edited

Loading

clawsweeper Bot commented Sep 5, 2026 •

edited

Loading