[freeqaz]

Problem

Dogfood finding D707: a member with Configuration update but no Secret operate got enabled Channel Secrets pickers and an enabled Save channel Secrets. The save then answered 403 (PATCH Configuration).

The gap is wider than Channel Secrets. updateConfiguration authorizes Secret operate on every Secret the saved Configuration binds, including bindings the request leaves unchanged. So for this member, every Configuration save fails while the Configuration binds a Secret they cannot operate: the JSON editor, the Channels drawer and Channel Secrets. The caller-permissions answer from #3600 reports only Configuration update, so the console could not tell.

Change

API (additive): GET /namespaces/{namespaceId}/agents/{agentId}/permissions adds configurationSecretsOperate.

• It is true when the IAM Driver allows Secret operate on every Secret the Agent's Configuration binds now, and true when the Configuration binds none.
• It is answered only to a caller with Configuration read and update; anyone else gets false.
• Like the other bits, it is advisory and audits nothing.
• A new OCC method, getConfigurationSecretSources, returns the distinct bound Secret references from the Configuration record (a Secret bound under two names is one IAM check). It requires Configuration read and does not read the document through the Configuration Driver. Unlike getConfiguration it skips the provisioning guard: saves are refused while an Agent is provisioning, so the answer enables nothing then, and the guard's 409 would fail the whole permissions answer.
• Updated the contracts schema, the OpenAPI document, docs/reference/api.md and the console guide.

Console: configurationSecretsOperate: false makes the draft's Configuration, Channels and Channel Secrets read-only, the same views #3600 made follow configurationUpdate. Each one names the missing grant: "Saving this Configuration needs Secret operate on each Secret it binds, which your access does not include." Server authorization is unchanged. The Channel Secrets read-only texts have one copy in credentials.mts. The console flow doc (agent-editing.md) says a missing bound Secret operate also makes the draft read-only.

Why this answer and not the other options

• A Namespace-level "may bind Secrets" bit: the IAM model has no exact Namespace-level answer. Grants can be scoped to one Secret through access bindings, and restrictions can deny one Secret. So a Namespace probe would be wrong in both directions.
• Per-Secret answers on the Secret list: this would change the shared Secret list response for every picker and the Secrets page, with an IAM call per listed Secret. It also leaves the Configuration editor and Channels without an answer.
• The chosen answer covers exactly what every save checks for the bindings it keeps. It is scoped to one Agent's Configuration, and it reveals nothing new: the caller already reads the bound Secret references (Configuration read), and with Configuration update any save attempt reports the same decision.

Verification

• tests/integration/console-api.test.mjs, "caller permission summaries…", covers the answer per persona:
  • admin: true.
  • Agent read+deploy only: false.
  • Configuration update without read: false.
  • read+update without operate on the bound Secrets: false.
  • exact operate on one of the two bound Secrets: false (an "any instead of every" mutant now fails here), and a resave of the same values as that caller answers 403.
  • exact operate on both: true, and the same resave answers 200.
  • No authorization denial is audited for the permission reads.
• New browser test in tests/browser/console-agent-detail.test.mjs, "a Configuration editor without Secret operate on its bound Secrets reads them". On a Slack Agent with both tokens bound:
  • The Channel Secrets pickers are disabled, with the reason and no Save channel Secrets.
  • The Configuration tab has no Edit Configuration and names the grant.
  • Channels is read-only.
  • No write is sent.
• Both tests fail on main (db913e95e8). Two mutants also fail them: the server answering true regardless of the bound Secrets, and the console ignoring the bit.
• On a remote test VM in a private network namespace:
  • Eight browser files plus console-api and generated-api-reference passed 333/333. The browser files are detail, detail-2, save refusals, saves, channels, runtime credentials, agents and agents-2.
  • The new browser test pinned 20/20 at 2 CPUs.
• After syncing with main (fix(console): show plugin selections and the current version read-only to a reader #3608, fix(console): drop the bound slack setup hint, describe disabled secret pickers and widen channel secrets #3609, test: hold console requests through the shared holdRequests helper #3610, test: build faked API error answers with a contract-checked helper #3612): the Stop/Delete negative-answer assertion from test: hold console requests through the shared holdRequests helper #3610 includes configurationSecretsOperate: false, and fix(console): show plugin selections and the current version read-only to a reader #3608's "Edit current Configuration" / "Open version draft" entry follows the new bit. console-api passes 10/10 and its permission test pinned 20/20 at 2 CPUs; the seven Configuration access browser tests pinned 20/20 at 2 CPUs; the checks-browser lane passed on a remote test VM. agent-editing.md is 2,489 words by the repo's counter.
• pnpm openapi:check equivalent is current. tsc --build, eslint and prettier are clean on the touched files.
• Storybook: new story Components/Credentials, "Slack tokens, editor cannot operate the bound Secrets". I did not build Storybook locally because its dependencies aren't installed in this checkout; CI builds it.

Screenshots are from the console test fixture at 1440 px: a member with Configuration read+update and Secret read, no Secret operate, on a Slack Agent with both tokens bound. They are simulated UI proof. The picker hint text is fixed separately in #3609.

	Before	After
Channel Secrets
Configuration tab

Risks

• Read-only is stricter than the API in one case: a member could replace every Secret they can't operate with Secrets they can, in one Channel Secrets save. The console now shows that panel read-only. Recovery is to ask for operate on the bound Secrets.
• Not covered: a binding that is missing today and then filled with a Secret the caller can't operate is still refused only on save (the existing message names Secret operate).
• Cost: the permission read now does one Configuration record read plus one IAM check per bound Secret, but only for callers with Configuration read and update.
• No upgrade notice: the change only adds a response field, like fix(console): show the version draft read-only to a caller who can only read it #3600.
• Agent saves are not covered (follow-up): every Agent PATCH also requires Secret operate on each Secret the Configuration binds. A caller with Agent update and Configuration read but no operate on a bound Secret still gets enabled plugin, authentication and repository editors whose Save answers 403. This bit is answered only with Configuration update, so the console cannot reuse it for those saves; tracked separately.

[clawsweeper]

🦞👀
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]

Codex review: needs changes before merge.

What this changes

This PR makes Configuration editing read-only when the caller cannot operate every currently bound Secret.

Example: A member has Configuration read and update, but lacks Secret operate on the bound SLACK_APP_TOKEN and SLACK_BOT_TOKEN.

• Before: The Console offers editing controls whose Configuration save returns 403.
• After: Configuration, Channels, and Channel Secrets show read-only controls with a missing-permission explanation.

Review scores

Measure	Result	What it means
Overall readiness	🦐 gold shrimp (3/6)	The focused implementation matches the existing authorization contract; visual verification is incomplete and two small corrections remain.
Proof confidence	🦐 gold shrimp (3/6)	Real behavior proof is not required for maintainer- or bot-authored pull requests.
Patch quality	🐚 platinum hermit (4/6)	2 actionable review findings remain.

Product

Kind: Bug fix · Worth it: Yes · Fix scope: Complete
User problem: A member is offered Configuration editing even though retaining its bound Secrets makes the save fail with 403.
Reason: The report matches the current save contract, and the collaborator-authored PR records the intended conservative UI behavior.

Merge readiness

⛔ Needs changes before merge - 2 items remain

Keep open: this is a useful, maintainer-directed fix for a permission mismatch still present on main.

Priority: P2
Reviewed head: 3b8a72f2fa98666e5ff4018b4398e409a2949daa

Before merge

• Tell denied editors how to obtain the missing access (P3) - When configurationSecretsOperate is false, the new notices remove editing controls and identify the missing permission, but give the member no next action. Add the recorded recovery instruction—ask an Installation administrator for Secret operate on the bound Secrets—to this notice and the Channel Secrets variant. This satisfies the review's actionable-message requirement without changing the accepted gate.
• Update the owning flow for the bound-Secret save gate (P3) - The new runtime branch reads Configuration metadata and probes each bound Secret before deciding whether editors are available, but docs/flows/platform-console/agent-editing.md still describes only Configuration update as the read-only gate. Update that existing flow with configurationSecretsOperate, its read/update disclosure guard, and unchanged save-time authorization. AGENTS.md requires affected flows to accompany behavior-changing implementations.

Findings

• [P3] Tell denied editors how to obtain the missing access — apps/controller/src/console/agents/detail.mts:338-339
• [P3] Update the owning flow for the bound-Secret save gate — apps/controller/src/index.ts:2420-2426

Tests

• Missing end-to-end proof: Supply the required short UI walkthrough and verify the new story from rebuilt Storybook source; the body reports base-failure and mutant checks, which this review did not rerun.

Agent review details

How this fits together

The Console consumes an Agent permission summary from Fastify; OCC reads bound Secret references, and the selected IAM Driver supplies advisory decisions while save operations retain their own authorization.

flowchart TB
  Member[Namespace member] --> Console[Agent Console]
  Console --> API[Permission summary API]
  API --> OCC[OCC Configuration metadata]
  OCC --> Bindings[Bound Secret references]
  API --> IAM[Selected IAM Driver]
  IAM --> API
  API --> Console

Loading

Technical review

Best possible solution:

Keep the accepted advisory gate, add actionable access-recovery guidance, and update the existing Agent-editing flow.

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

The current-main editor gate ignores Secret operate, while updateConfiguration authorizes every retained binding; the supplied browser case covers that exact persona, but no tests were run during this read-only review.

Is this the best way to solve the issue?

The existing permission-summary owner is an appropriate place for this advisory answer, and the maintainer explicitly accepts its conservative treatment of replacement bindings.

Full review comments:

• [P3] Tell denied editors how to obtain the missing access — apps/controller/src/console/agents/detail.mts:338-339
  When configurationSecretsOperate is false, the new notices remove editing controls and identify the missing permission, but give the member no next action. Add the recorded recovery instruction—ask an Installation administrator for Secret operate on the bound Secrets—to this notice and the Channel Secrets variant. This satisfies the review's actionable-message requirement without changing the accepted gate.
  Confidence: 0.98
• [P3] Update the owning flow for the bound-Secret save gate — apps/controller/src/index.ts:2420-2426
  The new runtime branch reads Configuration metadata and probes each bound Secret before deciding whether editors are available, but docs/flows/platform-console/agent-editing.md still describes only Configuration update as the read-only gate. Update that existing flow with configurationSecretsOperate, its read/update disclosure guard, and unchanged save-time authorization. AGENTS.md requires affected flows to accompany behavior-changing implementations.
  Confidence: 0.99

Overall correctness: patch is correct
Overall confidence: 0.94

AGENTS.md: found and applied where relevant.

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

Provenance checked

• Advisory permission summaries keeps the original intent (be5deb2: fix(console): explain unavailable Agent actions before confirmation #836 explains unavailable actions before confirmation while retaining actual-operation authorization and the normal flow when hints are unavailable.)
• Configuration, Channels, and Channel Secrets read-only views keeps the original intent (aaabca1: fix(console): show the version draft read-only to a caller who can only read it #3600 prevents readers from entering editors whose saves are refused; this PR extends those existing views to another save prerequisite.)
• Conservative blocking of replacement bindings changes intended behavior with a stated reason (aaabca1: The previous gate allowed Configuration editors to replace bindings; this PR explicitly explains why its current-binding hint also blocks a replace-all save.)

Testing

Proof path: in-process harness.

Security

None.

Evidence

Acceptance criteria:

• node --test --test-name-pattern='a Configuration editor without Secret operate' tests/browser/console-agent-detail.test.mjs
• pnpm docs:check
• pnpm docs:check-length
• pnpm lint
• pnpm format:check

What I checked:

• Main still lacks the bound-Secret hint: Current main reports configurationUpdate alone and gates Configuration editing on that bit; it does not report configurationSecretsOperate. (apps/controller/src/index.ts:2408, c000db1d561d)
• Save authorization matches the reported problem: updateConfiguration preserves omitted bindings and calls authorizeBindings before advancing the Configuration; authorizeBindings checks exact Secret operate for every retained source. (packages/occ/src/index.ts:7063, 3b8a72f2fa98)
• Introduced change and disclosure boundary: The pinned introduced diff adds the advisory answer only after Configuration read and update succeed, reads references rather than Secret values, and leaves save authorization unchanged. (apps/controller/src/index.ts:2420, 3b8a72f2fa98)
• Maintainer-directed tradeoff: The collaborator-authored body explicitly accepts blocking a Console save that replaces every inaccessible binding, although the API could authorize its replacement bindings; no additional direction approval is needed. (3b8a72f2fa98)
• Inspected visual evidence: All four downloaded before/after images were inspected. They show the changed Channel Secrets and Configuration views using the Console fixture; the manifest contains no video, and the body states Storybook was not built locally. (3b8a72f2fa98)
• Existing flow omits the new gate: The Agent-editing flow describes read-only behavior for missing Configuration update but does not describe the new bound-Secret decision or its metadata-only lookup. (docs/flows/platform-console/agent-editing.md:117, 3b8a72f2fa98)

Labels

Label changes:

• add P2: This prevents predictable permission-denied saves for a limited Console persona without changing server authorization.
• add merge-risk: 🚨 compatibility: Existing editors lose the Console's previously valid replace-all-bindings recovery path; the maintainer-authored body explicitly accepts that narrower UI behavior.
• add rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• add status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Label justifications:

• P2: This prevents predictable permission-denied saves for a limited Console persona without changing server authorization.
• merge-risk: 🚨 compatibility: Existing editors lose the Console's previously valid replace-all-bindings recovery path; the maintainer-authored body explicitly accepts that narrower UI behavior.
• rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦐 gold shrimp and patch quality is 🐚 platinum hermit.
• status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action.

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Complete the scoped UI verification described in realBehaviorProof.

Rating scale

6/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.

Workflow

ClawSweeper edits this one comment on every review. Comment @clawsweeper re-review for a fresh review only. Repair and merge execution lanes are retired.

Reviewed October 25, 2026, 6:22 AM ET / 10:22 UTC.