Repository navigation
fix(console): read the default slack account's token keys and show channel refusals on credentials - #3686
Conversation
…annel refusals on credentials The Credentials tab asked for SLACK_APP_TOKEN and SLACK_BOT_TOKEN whenever there were no named accounts, so a default account whose tokens name other keys, or HTTP mode with no app token, held Deploy for a Configuration that save and the Slack Channel Driver accept (finding 2526). It now reads each token's key as the driver does and lists those keys without pickers when they are not the standard pair. A channel Secret save resends the stored values, so a stored token form that save now refuses got a 400 naming the setting, which the tab replaced with "Check the selected Secrets and reload this Agent." (finding 2529). It now shows the API's message and points to the Configuration JSON.
|
🦞👀 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. |
|
Codex review: needs maintainer review before merge. What this changesThe Console reads Slack’s configured credential keys, preserves grant checks across saves, and shows actionable channel-save refusals. Example: Slack uses ${ACME_SLACK_APP} and ${ACME_SLACK_BOT}, with both Secrets bound and granted.
Review scores
ProductKind: Bug fix · Worth it: Yes · Fix scope: Complete Merge readiness✅ Ready for maintainer review Keep this PR open: it fixes credential-readiness bugs still present on main, and the prior findings are addressed; no remaining code defect was found. Priority: P2 Before mergeNone. FindingsNone. Tests
Agent review detailsHow this fits togetherThe Agent Credentials panel reads saved Configuration bindings and Namespace IAM policy, displays missing bindings or grants, and gates deployment requests; OCC and the worker retain final authorization. flowchart TB
A[Saved Configuration] --> B[Credentials panel]
C[Namespace IAM policy] --> B
B --> D[Binding rows and Deploy gate]
B --> E[Exact Secret grant API]
D --> F[Deployment API]
F --> G[Worker authorization]
E --> C
Technical reviewBest possible solution: Land the existing credential-panel repair after its corrected final UI has reviewable visual evidence. Do we have a high-confidence way to reproduce the issue? Current main directly shows the fixed-key readiness check; supplied browser regressions and recorded reproductions establish the grant-read races, but this review executed no target code. Is this the best way to solve the issue? The patch repairs the existing Credentials owner, preserves authoritative server authorization, and adds meaningful coverage for missing bindings, grant recovery, refusal messages, and both save/read orderings. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against cadab7afee96. Provenance checked
TestingProof path: in-process harness. SecurityNone. EvidenceWhat I checked:
Likely related people:
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 HistoryReviewed October 29, 2026, 7:25 AM ET / 11:25 UTC (Revision 3). |
…token reference
Review findings on this PR:
- 2540: a default account whose tokens read only standard keys (HTTP mode with
botToken ${SLACK_BOT_TOKEN}) keeps pickers for just those keys instead of the
custom-keys list, so the bot token can still be rebound in the console. The
other-keys hint no longer says the listed keys are not the standard pair.
- 2541: in HTTP mode the tab also lists each name a string app token references
($NAME, ${NAME:-x}, xapp-${NAME}), as save requires a binding for each.
- 2542: a save refused for a reference without a binding gets where the console
binds one (shared with the Agent page editor's remedy), not the Configuration
JSON pointer.
# Conflicts: # apps/controller/src/console/environment-references.d.mts # apps/controller/src/console/environment-references.mjs
freeqaz-openai
left a comment
There was a problem hiding this comment.
I reproduced two cases on 115df49e using the CI-built Storybook and synthetic Secret bindings.
- Deploy becomes available when the Agent still lacks access to a bound Secret. In the “Slack Secret bound without an Agent grant” story, leave the Agent without its grant for
SLACK_APP_TOKEN. Keep that app binding and change the bot reference to${ACME_SLACK_BOT}with its binding present. Both rows now say Bound, Retry Secret access disappears, and Deploy new version becomes enabled. With the standard bot key, the app row shows Agent access not confirmed and Deploy stays disabled. IncheckBoundSecretGrants, deriving the read fromslackFormBindings(...) ?? []skips the entire check when any key requires the read-only list. Please check the Secrets the default account actually references, independently of which pickers it can show, and keep the missing-grant warning and recovery available. - Swapping the two standard references reverses what the picker labels mean. Set
appTokento${SLACK_BOT_TOKEN}andbotTokento${SLACK_APP_TOKEN}. The form still shows both role-labelled pickers, but the Slack app token picker writes the key the bot reads. Please compare each token role with its effective binding before offering that picker. The existing read-only list is a suitable fallback. The ordinary HTTP configuration with only a standard bot token should keep its bot picker.
I also opened the custom-key and HTTP previews and pressed Save channel Secrets in the refusal preview. The alert received focus and named the setting and the Configuration JSON repair location.
…heck grants for listed keys Review on 115df49: - A default account whose tokens read each other's standard key (appToken ${SLACK_BOT_TOKEN}, botToken ${SLACK_APP_TOKEN}) got role-labelled pickers that bound the other token's key. The form now requires each token to read its own standard key; other shapes get the list. HTTP mode with only a standard bot token keeps its bot picker. - In list mode the administrator's grant check was skipped, so a listed key whose Secret the Agent may not use showed Bound and Deploy was enabled. Grant checks and pending grants now cover every listed key, and list mode keeps a Retry Secret access button for them. - The other-keys hint now states what the pickers bind and that the tokens read the listed keys, which holds for every list shape.
# Conflicts: # apps/controller/src/console/agents/credentials.mts # tests/browser/console-runtime-credentials.test.mjs
|
Thanks, both reproduced and fixed in 7aa5946 (head d267ee8, which also merges main).
New browser cases in |
…ference save requires Deploy checks that the Agent may operate every bound Secret, but the Credentials tab checked grants only for the keys the Slack tokens read, so a leftover SLACK_APP_TOKEN in HTTP mode (or the standard pair beside custom token keys) showed Ready to deploy with no Retry and the deploy failed AUTHORIZATION_DENIED (finding 2558). Grant checks, outstanding grants and Retry now cover every bound Secret, and each bound key no Slack setting reads gets its own row. The tab also listed only the tokens, while save refuses an unbound signingSecret, userToken or relay.authToken reference (finding 2554). It now lists every name save requires a binding for.
freeqaz-openai
left a comment
There was a problem hiding this comment.
I verified the two reported corrections on d267ee8. Crossed token references now show a read-only list. With a custom bot key and a missing app-Secret grant, the list shows the warning, offers Retry, and disables Deploy. Retry sent only the expected grant request and then enabled Deploy.
One timing case still sends a deployment request after the Console knows the grant is missing. In the CI-built Storybook I held the grant reads, clicked Deploy new version, and paused the click handler's Configuration reread. I then released the grant reads and observed Bound · Agent access not confirmed on the app row. Releasing the Configuration reread sent a POST to the simulated Agent deployment endpoint.
Please keep Deploy waiting while its relevant grant read is in flight and check the credential panel's current readiness again immediately before the POST in apps/controller/src/console/agents/detail.mts. A regression test can control the response order above and assert zero Deploy requests. Keep the positive control where Retry repairs the grant and Deploy becomes available. This is a Console request-ordering finding; the reproduction used the fixture backend.
…a leftover key and a signing secret Storybook gains Slack key left bound without an Agent grant (HTTP mode with SLACK_APP_TOKEN still bound) and Slack signing secret without a binding. Channel Secrets rows can now hold long environment keys, so the label column grows with them instead of overlapping the badge.
…edentials before the deploy request Review on d267ee8: with the grant reads held, Deploy new version was enabled, and a click whose Configuration reread outlasted them sent the deploy request after the panel showed the missing grant. Deploy now waits for the Credentials panel's grant read (and the harness credential's), and the click asks the panel again just before the POST.
One message for both grant reads: Checking this Agent's credential access before deploying. Storybook gains Slack grant read in progress.
# Conflicts: # apps/controller/src/console/agents/credentials.mts
|
Thanks, reproduced and fixed in this push (head 8a1a940, which also merges main through #3693). Your timing case. Deploy now waits for the grant read:
Also in this push (internal re-review of d267ee8 found these):
All new and changed cases pass 20/20 at 2 CPUs. Before/after screenshots and videos are in the PR body. |
|
🦞👀 Re-review progress:
|
freeqaz-openai
left a comment
There was a problem hiding this comment.
On ffddb8e28c, the Deploy hold now works. In the CI-built Storybook, changing only the bot Secret while the first reads were pending sent zero deployment requests with the app grant missing and one with it present.
I also exercised the initially granted case against the real Controller in the local browser fixture on the CI-tested tree. Releasing the original read before or after the save gave the same result: one deployment POST, HTTP 202, and one stored revision. Please keep this positive assertion alongside the missing-grant case in the owning test.
Please fix the success message immediately above Retry Secret access. After all reads settle, the missing-grant case says both “deploying it would be refused” and “Channel Secret bindings saved. Deploy the new version to deliver them.” The latter comes from credentials.mts:1126, and the replacement read updates the warning and rows while leaving that success message visible. “Channel Secret bindings saved.” is enough when a grant recheck is pending. Please assert that the settled missing-grant state offers Retry without directing the user to Deploy. The existing Claw review also covers the HTTP picker guide and the remaining deployment-test coverage.
# Conflicts: # apps/controller/src/console/agents/detail.mts # docs/flows/platform-console/agent-editing.md # tests/browser/console-runtime-credentials.test.mjs # tests/helpers/refusal-text.mjs
…ss is confirmed
A save that ended with a grant read still to come, or with a bound Secret
still missing its Agent grant, said "Deploy the new version to deliver them"
above Retry Secret access while the deploy gate held Deploy. The save
message now says only "Channel Secret bindings saved." (or the Retry
equivalent) until the read settles with nothing missing.
The owning browser test asserts the settled missing-grant state offers Retry
without pointing to Deploy, and adds the granted positive control: one deploy
POST, HTTP 202, one stored version. The guide qualifies the HTTP bot-only
picker sentence: HTTP mode with appToken "${SLACK_APP_TOKEN}" shows both.
… grants with a channel runtime The positive control deploys, so its fixture evaluates the grants the console commits through the IAM API and admits Slack with an Installation channel runtime. The deploy status assertion carries the API's answer on failure. The short save message is asserted at save end only for the missing-grant cases: with the grant present, the save's unheld recheck may already have settled and named Deploy.
… the main merge Main brought the page to 2498 words and this PR's Credentials trace pushed the merge to 2690. Condense the channel Secret grant paragraph and the step 5 grant note, fold this PR's changelog lines into one, and move the older dated entries to the history page. The docs word counter reports 2495.
|
Free's clanker here (an AI agent working on Free's behalf). This push addresses the review on Merge with main. The branch conflicted with main; it now includes main through Success message above Retry Secret access ( Owning test (
Guide ( Verification (a remote test VM, browser tests in a private network namespace):
|
Verified on e2083d3: the missing-grant cases keep Retry available without directing the operator to Deploy. In both response orders, the granted cases assert one deployment POST, HTTP 202, and one matching new revision. All four passed in the hosted credentials suite. The guide now distinguishes the two HTTP picker configurations. This resolves the findings in this review.
…t of the agent editing flow The main merge brought back three changelog entries that this branch had already moved to agent-editing/history.md, putting the page at 2543 words. Without the duplicates it is 2496.
|
Free's clanker here (an AI agent working on Free's behalf). I synced this branch with main; head is now Merge with main (
Docs follow-up ( Verification on the merged tree (remote test VM): Media: the screenshots and video in the body predate the 10-27 text-only change to the success message above Retry Secret access. |
|
@clawsweeper re-review |
|
🦞👀 Re-review progress:
|
Problem
Two gaps in the Agent page's Credentials tab, both next to the 10-25 Slack token rules (#3680, #3681):
missingChannelCredentialGroupsalways asked forSLACK_APP_TOKENandSLACK_BOT_TOKEN. It ignored the keys the default account's tokens actually name and ignored HTTP mode, which reads no app token. A Configuration withappToken: "${ACME_SLACK_APP}"andbotToken: "${ACME_SLACK_BOT}", both bound, is accepted by save and by the Slack Channel Driver, but the tab reported "Slack Secret bindings" missing and disabled Deploy new version. HTTP mode with a bound bot token and no app token had the same result.SLACK_APP_TOKENstill bound from Socket Mode and no Agent grant on it (or the standard pair left bound beside custom token keys), the tab said "Ready to deploy." with no Retry, and the deploy failedAUTHORIZATION_DENIED. The tab also listed only the tokens, so Deploy was available while save and deployment refuse an unboundsigningSecret,userTokenorrelay.authTokenreference.SLACK_APP_TOKENbound and ungranted, repointing only the bot token during the read granted only the new bot Secret: both rows showed Bound, the tab said "Ready to deploy.", and the deploy failedAUTHORIZATION_DENIED. The "Checking this Agent's credential access" status also flashed on every administrator page open.Change
credentials.mts: with no named accounts, the tab reads each default-account token's key as the Slack Channel Driver does: a whole${NAME}or an env SecretRef id. A token that names no key keeps the standard name. Deploy waits until each key is bound. The tab never lists fewer token bindings than save requires.SLACK_APP_TOKENorSLACK_BOT_TOKEN, the form shows a picker for each of those keys only. In HTTP mode withbotToken: "${SLACK_BOT_TOKEN}"and no app-token reference, the tab keeps the Slack bot token picker alone, which is the only console path that rebinds it. When any key is another name, the tab lists the keys as Bound or Missing without pickers and points to the Configuration API, as it already did for named accounts. The hint no longer says the listed keys are not the standard pair.$NAME,${NAME:-text}andxapp-${NAME}included. The tab lists those names (environmentReferenceNamesandsecretInputShorthandName, now served to the console withunsubstitutedReference).appToken: "${SLACK_BOT_TOKEN}") get the list, so no picker binds a Secret under the wrong token's label.requireChannelSecretBindingsreads them: env SecretRef ids anywhere,${NAME}without a fallback, and in secret-input fields (appToken,botToken,signingSecret,userToken,relay.authToken, on the channel and on each named account) also${NAME:-text}and$NAME. Keys without a picker, such asSLACK_SIGNING_SECRET, are listed below the pickers and Deploy waits for them.detail.mts, so a grant read that lands during its rereads stops the request.delayed-statusspan, so a read that settles sooner shows nothing. Deploy stays disabled during the read.channelBindingRefusalmatches shows the API's message plus where to fix it. A refusal that asks only for a binding ("… without a Configuration Secret binding") gets where the console binds one, the same text the Configuration JSON editor uses (CHANNEL_BINDING_PLACESinlist.mts, now shared). Other refusals get "Fix that setting in the Configuration JSON on the Configuration tab: this tab saves only the SLACK_APP_TOKEN and SLACK_BOT_TOKEN bindings." The message names the setting and never contains token text. Other 400s keep the old text.${ACME_SLACK_BOT}), Slack HTTP mode, bot token only, Slack key left bound without an Agent grant, Slack signing secret without a binding, Slack grant read in progress, Channel Secret save refused for a channel setting and Channel Secret save refused for a missing binding. The fixture gainedslackTokensand a list form ofslackBindings.channels-and-credentials.md, and the console editing flow.Verification
tests/conformance/slack-configuration.test.mjs: custom${NAME}keys, a mixed env SecretRef plus a missing token, HTTP mode with no app token, and HTTP-mode app tokens written as${NAME},${NAME:-x},xapp-${NAME},$NAME,$SLACK_APP_TOKENand two references.tests/browser/console-runtime-credentials.test.mjs, four cases: (1) the default account's${ACME_*}keys are listed Bound with the corrected hint, there are no pickers, and Deploy is enabled; (2) HTTP mode shows only the bot token picker, a rotation saves and Deploy stays available; (3) a stored file SecretRef bot token gets its refusal with the Configuration JSON pointer; (4) a stored unboundsigningSecretreference gets its refusal with the binding remedy and no Configuration JSON pointer. (3) and (4) write the stored row through the storage Driver the way an earlier build stored it, under the bundled Kubernetes Compute rule, and get a real 400. Nothing is saved.de828d999c, the same branch without the timing fix.8a1a940e3fwith "Ready to deploy.". The held-read case also checks that the checking text sits in the delayed-status span (fails on8a1a940e3f). The three cases pass 20/20 at 2 CPUs on a remote test VM in a private network namespace. Onffddb8e28c(main merged incl. fix(console): show the API's typed refusal reasons through one shared describer #3690, fix(deploy): give every deployment failure code a message and a next step #3697), the credentials, channels, draft-guards, detail, detail-2 and overview browser files plus the Slack conformance file pass 315/315. The 51-shape probe output is unchanged.tests/browser/console-runtime-credentials.test.mjs: (5) a listed key without the Agent's grant, (6) HTTP mode withSLACK_APP_TOKENleft bound and ungranted, and (7) custom token keys with the standard pair left bound andSLACK_APP_TOKENungranted: each marks the row, holds Deploy, and Retry grants exactly that Secret with no Configuration write; (8) asigningSecretreference without a binding is listed Missing and holds Deploy; (9) swapped standard keys get the list and no pickers, checked before any copy. The HTTP-mode case now also expects theSLACK_SIGNING_SECRETrow.tests/conformance/slack-configuration.test.mjs: for seven shapes (signing secret, user token, relay token, fallbacks inside and outside secret-input fields, an env SecretRef in another field, named accounts), the test collects every name the real save rule requires, leaves each one unbound in turn, and expects the gate to hold Deploy.d267ee8de6(5 browser cases, the conformance case), and the swapped-keys case fails on115df49efcat the picker assertion. All pass on this head (8a1a940e3f, main merged incl. fix(console): keep deploy open after a member's credential save and name bound secrets a deployer cannot operate #3692): the credentials, draft-guards, channels, detail, saves, drafts-kept and deploy-again browser files plus the conformance file 267/267; a wider run of 16 console browser files passed 461/462, the one failure a 250 ms layout-timing case under 8-way load that passes 6/6 on its own.d267ee8de6). A browser probe of 14 grant shapes (standard, swapped, custom, HTTP, named accounts, leftover keys, signing secret, user token, relay token) holds Deploy whenever a bound Secret lacks the Agent's grant, and Retry grants exactly that Secret. Two shapes with every grant in place stay deployable with no grant writes.tsc --build(console and tests), eslint, and prettier pass on the touched files.Media at
f0288e4a09(this head). Console Storybook, Components/Credentials, rendered fromprepare-assets.mjsoutput in headless Chromium: simulated UI proof (simulated API; no backend persistence, credential propagation or deployment). Before = main at the merge basebe63cc7e19with this PR's stories and fixtures copied in, so the same scenario renders on main's console code. After = this head. 1440 px full page, 390 px the Channel Secrets section (and the Prepare card where Deploy matters). These replace the earlier captures, which predate the text-only save message (132248437f) and main's Agent detail layout change. The browser tests above cover the real console and API.A save during the grant read (story Slack grant read in progress: the policy read is held 4 s; the bot token is switched to the backup Secret and saved while the read runs; the app token Secret stays bound without the Agent grant).


Before (main): both rows Bound, "Ready to deploy." and Deploy enabled while the app grant is missing.
After: the app row reads Bound · Agent access not confirmed, Retry Secret access is offered, the save says only "Channel Secret bindings saved." and Deploy waits with "Resolve the saved Secret access grant before deploying."



After Retry Secret access: "This Agent can use its saved Slack Secrets. Deploy the new version to deliver them." and Deploy is available. No deploy request was sent during the capture.

Slack key left bound without an Agent grant (HTTP mode,


SLACK_APP_TOKENstill bound and ungranted). Before (main): both standard pickers, with the app token treated as read.After: the bot token picker alone and a


SLACK_APP_TOKENrow, Bound · not read by the Slack settings · Agent access not confirmed; Deploy is held and Retry is offered.After Retry Secret access: the row reads Bound · not read by the Slack settings and Deploy is available.

Slack signing secret without a binding. Before (main): "Ready to deploy." although save and deployment refuse it.


After:


SLACK_SIGNING_SECRETis listed Missing below the pickers and Deploy waits for it.Slack grant read in progress (no interaction, captured while the read is held). Before (main): "Ready to deploy." and Deploy enabled.


After: "Checking this Agent's credential access before deploying." and Deploy disabled until the read lands.


Slack HTTP mode, bot token only. Before (main): the app token is listed Missing, Deploy is disabled and Save stays disabled.


After: the bot token picker alone and "Ready to deploy."; rotating it to another Secret saves.



Slack tokens on other keys (bot token


${ACME_SLACK_BOT}, unbound). Before (main): the standard pair with pickers, and Deploy names no key.After: the keys the tokens read,


ACME_SLACK_BOTMissing, no pickers, the Configuration API hint, and Deploy waits forACME_SLACK_BOT.Channel Secret save refused for a missing binding. Before (main): "Check the selected Secrets and reload this Agent."


After: the API message plus where the console binds a Secret, with no Configuration JSON pointer.


Channel Secret save refused for a channel setting. Before (main): the same generic text.


After: the API message plus the Configuration JSON pointer.


Videos at
f0288e4a09, 1440 px: save during the grant read, then Retry, leftover key, Retry Secret access, HTTP-mode bot token rotation, refused save for a missing binding, refused save for a channel setting.Known at 390 px: the status badges do not wrap, so a long one runs past the card (on main too, for "Bound · Agent access not confirmed"); the new "Bound · not read by the Slack settings · Agent access not confirmed" badge widens the page. Tracked separately.
Risks
${ACME_SLACK_BOT}, loses the pickers on this tab and gets the named-account treatment: bind through the Configuration API. Before this change those pickers bound keys the tokens did not read.SLACK_APP_TOKEN, or a binding another setting uses), because deploy checks their grants too.SLACK_SECRET_INPUTSin the console mirrors the OCC list inchannel-secret-bindings.ts; the conformance test runs the real save rule, so a drift that lists fewer names fails it.$NAME, pasted value) with its names bound leaves Deploy enabled; the tab's save and deploy both refuse those and name the setting. Grant checks still run only while Slack is enabled.