Repository navigation
fix(console): grant Agent access to accepted Configuration Secrets - #2255
miguelbranco80 wants to merge 11 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: needs maintainer review before merge. What this changesThis PR grants a newly created Agent exact access to every distinct Secret referenced by its accepted saved Configuration. Example: An Embedded OpenClaw Preset binds a distinct tool Secret to both TOOL_TOKEN and SECOND_TOOL_TOKEN.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it repairs a current Console grant omission without changing the authorization contract. No actionable introduced defect was found, and GitHub no longer reports the earlier merge conflict. Priority: P2 Before mergeNone. FindingsNone. Agent review detailsHow this fits togetherThe Console saves Configuration and Agent resources through OpenClaw Control Plane, then delegates Secret access through existing IAM APIs; deployment admission and the worker independently enforce authorization before Kubernetes delivery. flowchart TD
Preset[Preset bindings] --> Console[Console creation]
Console --> State[Saved Configuration and Agent]
Console --> IAM[Exact Secret grants]
State --> Admission[Deployment admission]
IAM --> Admission
Admission --> Worker[Worker reauthorization]
Worker --> Kubernetes[Kubernetes Secret delivery]
Technical reviewBest possible solution: Land the focused saved-Configuration grant repair with existing IAM authorization and retained-Agent recovery intact. Do we have a high-confidence way to reproduce the issue? Current-main source retains inherited Configuration bindings but grants only staged channel selections, so a distinct inherited tool Secret lacks the Agent permission required by deployment admission; the contributor also records the regression failing against the unpatched Console. Is this the best way to solve the issue? Enumerating the accepted saved Configuration repairs the omission at its existing Console owner and reuses the established exact-grant operation without changing queued provisioning. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against fdde125f5e2a. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
Likely related people:
Review metrics
Root-cause clusterRelationship: Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. LabelsLabel changes:
Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (4 earlier review cycles)
Reviewed October 13, 2026, 9:21 AM ET / 13:21 UTC (Revision 5). |
clawsweeper
Bot
added
rating: 🦪 silver shellfish
clawsweeper
Bot
added
proof: sufficient
clawsweeper
Bot
added
status: 👀 ready for maintainer look
|
Independent source review of Should a Retry credential access grant reflect the Configuration accepted when the Agent was created, or the Configuration's current bindings at retry? The form retains the original create response (create.mjs). If an authorized actor replaces Secret A with B before a delayed grant or retry, this path can grant A and omit B; deployment admission still requires the current B grant. The grant caller must pass the current Installation-admin and Secret-read checks. The existing staged-channel path already uses the same saved snapshot; this PR extends that behavior to inherited and other prebound sources. I did not find a contract requiring Configuration membership to remain current when the IAM binding commits, or requiring existing grants to be revoked on a later Configuration edit. If the accepted snapshot is intended, no change follows from this observation. If current membership is intended, a pre-grant GET alone would still leave a race; the server-side transaction and retry contract would need to define the boundary. This is a source-derived scenario, not a demonstrated exploit or runtime failure. I did not independently rerun the browser, PostgreSQL, cluster, or provider evidence. The existing Maintainers review request remains in place. |
|
Free's clanker here. Update for Two useful recovery cases actually ran. When delegation was denied before a grant write, retry kept the one Agent and Configuration, granted exactly the model and bound tool Secrets, and left an unrelated Secret denied. The adjacent test covers the opposite ambiguity: a grant POST committed (201), its response was lost, and retry found the existing grant without creating another Agent. These are browser-fixture results, not a claim that a live Agent received a credential. The previously failing drawer-focus case also passed. No rerun needed for this check, no claim that the earlier flake is fixed, and auto-merge remains off as requested. Earlier result, head
PR #2938 already addresses the missing focus diagnostic for this exact failure. No need to copy that change here. I didn’t rerun CI, push to your branch, mark this ready, or enable auto-merge. Update: #2938 landed on I checked the nine changed paths against that |
|
Free's clanker here (an AI agent working on Free's behalf). One more review pass on head What it changes: when the Console creates an Agent from an accepted saved Configuration, it now grants the Agent Why I think it is safe
Risk: it is still an IAM grant change on the create path, which is why I'm flagging it rather than calling it routine. Nits (non-blocking)
Compatibility with the pending Credential Proxy work: it merges cleanly, and that work doesn't touch @freeqaz-openai flagging this one for your review. |
This branch has not been deployed
3 participants
Ordinary Console draft creation can save inherited Configuration Secret bindings without granting the Agent access, causing deployment admission to fail. Enumerate the accepted saved Configuration’s same-Namespace environment Secret bindings, deduplicate their sources, and create exact
operategrants through the existing IAM operation.Installation-admin delegation remains required. A failed grant retains the saved Agent and Configuration for explicit retry without duplicate writes. Supported queued provisioning keeps its worker-owned path. The existing live integration test also covers denied grant writes and revocation before worker delivery.
Fixes #2254. Fork tracking PR: raw-labs/openclaw-enterprise#13. Both use
raw-labs/openclaw-enterprise:codex/configuration-secret-access.Verification — October 20 refresh
Tested head:
7917bb951d8c7360bddcbbf3e4898445cc7e08ed, incorporating upstreammainata77594a92203b9da12690591554b385d9970d904(57 upstream commits since the previous base). Published history is preserved with a normal fast-forward push; both PRs remain open, non-draft, with auto-merge off.Preserved upstream's member permission gating, creation preflight, Credential Sources, Namespace polling, and Console changes. The parent Console flow now matches upstream exactly; the Configuration Secret grant trace follows upstream's move into
docs/flows/platform-console/agent-creation.md. Both consolidated changelog inventories and Manual Notes are preserved. Unrelated reference wording was restored. The remaining feature delta is nine files, +336/-50.One existing regression setup now revokes delegation after the page admits the administrator, matching upstream's retry scenario. A known non-admin still receives upstream's no-retry guidance. Real Native IAM denial, exact deduplicated grants, retained resource IDs, and revocation/admission assertions remain intact.
Local verification using matching prepared dependencies without installing anything:
a77594a9because the inherited tool Secret grant was missing.scoped-clean). The reviewer performed source review; test execution is reported separately above.Fresh hosted CI run 39407133353 is pending on this exact head. CodeQL and hosted Storybook have also started. Maintainer approval remains pending.
This refresh does not repeat live PostgreSQL/Kubernetes procedures. Their evidence remains scoped to earlier revisions below. Current media proves simulated Storybook UI behavior; it does not establish backend persistence, credential delivery, runtime readiness, or model execution.
Current Storybook evidence
Rebuilt and exercised Pages/Create Agent → Preset Secret access retry (
pages-create-agent--create-bound-secrets-grant-denied) on head7917bb951, macOS ARM64, Node.js 24.3.0. The actual retry opens the same saved Agent and Configuration. Both native screenshots opened at 1265×712; the uploaded H.264 recording decoded and played to completion (8.04 seconds).retry-walkthrough.mp4
Historical refresh
81ce5d06: 220 browser and 37 API cases passed; hosted CI subsequently passed. Its denial screenshot, recovery screenshot, and video remain historical.Earlier refresh
3cc28047: 119 browser and 37 API cases passed locally. Its inherited-Secret regression failed on exact unpatched upstreamf3a74292. Its denial screenshot, recovery screenshot, and video remain historical.Earlier live Console creation and admission
The remaining review request was exercised on
1d7772d11b2874ae7fb1f5253a998bd0463395b2using the actual loopback Console and HTTP listener, PostgreSQL application role, Native IAM, Kubernetes Configuration/Secret Drivers and controller worker. Setup provisioned a Namespace, a saved Embedded OpenClaw Preset and distinct synthetic model/tool Secrets. It did not seed an Agent or Agent Secret grant. The browser signed in, selected the Preset, entered name/model, selected Use Preset, Create Agent, then Deploy new version. No UI responses, State or IAM were mocked.Recorded real Console creation/admission output confirms:
TOOL_TOKENandSECOND_TOOL_TOKENbound to the tool Secret.operatebindings: one distinct model source and one inherited tool source, deduplicated across both destinations.create.mjsenumeration through the ordinary draft path; queued provisioning was not substituted.Created draft (real Console/State/IAM):
UI-admitted revision (real deployment record):
9.4-second recording of actual UI deployment admission:
live-console-deployment.mp4
Limit: An extra activation probe timed out; the worker recorded
DEPENDENCY_UNAVAILABLEbefore workload creation, and its cause was not diagnosed. This trace proves Console creation, exact inherited grants and admission; it does not establish runtime readiness. The separate four-case live fixture authority/delivery proof below remains passing. No genuine model turn or Helm-installed production deployment is claimed. The trace retains the activation failure, and only inspected, credential-free logs/media were uploaded. Owned fixtures have been removed. No repository/source changes were made for this follow-up.Earlier real-runtime verification
Tested head:
1d7772d11b2874ae7fb1f5253a998bd0463395b2. macOS ARM64, Node.js 24.3.0, pnpm 11.15.1, matching prepared dependencies, owned disposable PostgreSQL and loopback K3s fixtures with enforcing NetworkPolicies.All four live Kubernetes fixture cases passed, with zero failures/skips. Recorded authority proof and test output:
AUTHORIZATION_DENIED; the Agent has no active revision, Deployments or Pods, and no new Agent-owned Secrets are delivered in either execution or gateway placement.BOUND_SENTINELvalue inside the ready real gateway Pod. This includes the existing immutable revision, identity, isolation and teardown workflow.Commands:
node --env-file=<owned-fixture.env> --test tests/integration/kubernetes-compute-real.test.mjsandnode --env-file=<owned-provisioning.env> --test tests/integration/kubernetes-compute-provisioning-real.test.mjs tests/integration/kubernetes-compute-driver-real.test.mjs. Repository CI preparation migrates owned databases asocc_migrator; API/worker execution usesocc_app. The migrator seeds only the reader’s initial Role. Owned resources were cleaned up.These tests exercise actual Fastify routes through injection, OCC, Native IAM, PostgreSQL, the controller worker, and Kubernetes Drivers/API using development composition and a fixture gateway/Harness image. Synthetic values establish Secret delivery and refusal; they do not prove a genuine model turn, Slack connection or Helm-installed production deployment.
At the preceding head
e0ec6627, 93 Console browser and 54 API/OCC cases passed without failures/skips. The inherited-Secret regression failed against exact unpatched upstream Console and passed with the fix. This follow-up changes only the existing Kubernetes test; all application, browser-test, documentation and Storybook source is unchanged. Typecheck, OpenAPI and docs checks passed at that preceding head. Lint, formatting, workspace isolation and diff checks also pass at the new head. Existing reference/flow documentation remains accurate; no new runtime behavior or image version was introduced.Earlier UI evidence
Rebuilt and inspected Pages/Create Agent → Preset Secret access retry (
pages-create-agent--create-bound-secrets-grant-denied). The captures use UI source byte-identical to the tested head. Denial retains saved IDs and locks fields; selecting Retry credential access opens the same saved Agent draft. This is simulated Storybook UI proof, separate from the live authority proof above.Denied grant and saved-resource recovery:
Recovered retained Agent:
10-second recording of the live browser interaction; uploaded H.264 MP4 decoded and played successfully:
retry-walkthrough.mp4
The earlier evidence addresses the grant-persistence, post-admission revocation and media requests. Independent review comments are retained in the PR discussion. Keep both PRs open and auto-merge disabled; neither PR has been merged.