Repository navigation
fix(fe): gate read-only access UI behind a feature flag (default off) - #4087
Merged
Merged
Conversation
A read-only ("queries-only") delegation is a canister signature over a
message that includes `permissions = "queries"`. No released agent stack
(`ic-agent` / `ic-transport-types` / `@icp-sdk/core`) round-trips that
field: it is dropped when the delegation is re-serialized into the request
envelope, so the replica recomputes a different representation-independent
hash and canister-signature verification fails with "sig not found in the
signature tree". This broke read-only MCP connections (and would break any
read-only dapp/CLI session), since the delegation can't be used by current
agents.
Gate the access-level UI behind a new `READ_ONLY_MODE` feature flag, off by
default everywhere (including id.ai / beta.id.ai) until the agent stack
preserves the delegation `permissions` field:
- `AccessLevelToggle` is hidden in all three flows (`/continue`, `/cli`,
`/mcp`) when the flag is off.
- Each flow sends an `effectiveAccessLevel` that is forced to `full-access`
while the flag is off — so hiding the toggle can't leave `/cli` or `/mcp`
(which default to read-only) silently minting unusable delegations.
- e2e assertions updated to verify the toggle is absent by default.
The backend capability, candid `Permissions` type, and all delegation tests
are unchanged and stay in place; re-enabling read-only is a one-line flip of
the flag default once agents support the field.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR disables the read-only (“queries-only”) authorization UI by default via a new frontend feature flag, to avoid minting delegations that currently fail verification end-to-end because agent stacks drop the permissions="queries" field when re-serializing delegations.
Changes:
- Added a
READ_ONLY_MODEfeature flag (default off) with in-code documentation explaining the current agent incompatibility and the condition for re-enabling. - Hid
AccessLevelTogglein the/continue,/cli, and/mcpflows when the flag is off, and forced aneffectiveAccessLeveloffull-accessin those flows to avoid minting unusable queries-only delegations. - Updated Playwright e2e specs to assert the access-level toggle is not rendered by default.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/frontend/src/lib/state/featureFlags.ts | Introduces READ_ONLY_MODE flag (default off) and documents the rationale/flip condition. |
| src/frontend/src/routes/(new-styling)/authorize/views/ContinueView.svelte | Hides the access-level toggle behind the flag and forces full access when disabled. |
| src/frontend/src/routes/(new-styling)/cli/views/CliAuthorizeView.svelte | Hides the toggle behind the flag and ensures CLI authorizations are full access when disabled. |
| src/frontend/src/routes/(new-styling)/mcp/views/McpAuthorizeView.svelte | Hides the toggle behind the flag and ensures MCP connections request full access when disabled. |
| src/frontend/tests/e2e-playwright/routes/authorize/index.spec.ts | Asserts the read-only toggle is absent by default in the authorization flow. |
| src/frontend/tests/e2e-playwright/routes/cli.spec.ts | Asserts the CLI “Full access” toggle is absent by default. |
| src/frontend/tests/e2e-playwright/routes/mcp.spec.ts | Asserts the MCP “Read-only mode” toggle is absent by default. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review: the `accessLevel` default comments described the read-only / full-access defaults without noting they only apply when READ_ONLY_MODE is enabled. While the flag is off (the current default) the toggle is hidden and `effectiveAccessLevel` forces full access, so those defaults are unreachable. Clarified each comment accordingly. No behavior change.
aterga
marked this pull request as ready for review
July 3, 2026 17:09
|
✅ No security or compliance issues detected. Reviewed everything up to 185018e. Security Overview
Detected Code Changes
|
aterga
enabled auto-merge (squash)
July 3, 2026 17:22
basvandijk
disabled auto-merge
July 3, 2026 18:17
aterga
enabled auto-merge (squash)
July 3, 2026 18:18
aterga
disabled auto-merge
July 3, 2026 18:18
aterga
pushed a commit
that referenced
this pull request
Jul 4, 2026
…p-delegation-refactor-x5b6ek Conflicts were in the two files #4087 and this branch both touched: - McpAuthorizeView.svelte: adopt main's feature-flag gating. The access-level toggle now renders only under `{#if $READ_ONLY_MODE}` (default off) and `effectiveAccessLevel` forces "full-access" while the flag is off — so a connect never sends read-only ("queries") permissions until the flag is on, which is the intended behavior (no released agent round-trips a queries-only delegation yet). This supersedes this branch's prompt="full-access" tweak. Kept this branch's 30-day TTL preset/range (the MCP grant cap is 30 days). - mcp.spec.ts: keep this branch's session-key-model tests ("registers the server's session key", plus the finish_url test) and apply #4087's flag-off expectation — the toggle is hidden (toHaveCount(0)) and the completion POST reports permissions "all". The read-only→"queries" mapping stays covered by the utils unit tests, which drive mcpAuthorize directly. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VbjCCqTjsMn57fczTNGaKf
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Read-only mode (#4016) is broken end-to-end with today's agent stack. A read-only ("queries-only") delegation is a canister signature over a representation-independent hash that includes
permissions = "queries". No released agent (ic-agent/ic-transport-types, checked through 0.47.3;@icp-sdk/core) has apermissionsfield on itsDelegationwire type, and there is no#[serde(flatten)]catch-all — so the field is dropped when the delegation is re-serialized into the request envelope. The replica then recomputes the hash over{pubkey, expiration, targets}(nopermissions), which the II canister never signed, and canister-signature verification fails:This surfaced as MCP connections in read-only mode failing: the read-only grant makes the MCP server mint queries-only per-app delegations, which its agent can't use. The same fails-closed applies to read-only
/continueand/clisessions.Rather than revert the feature, gate the UI off until the agent stack preserves the delegation
permissionsfield (agent-side field preservation is enough to un-break verification; the field doesn't need to be semantically interpreted by the agent — enforcement stays the replica's job).Changes
READ_ONLY_MODEfeature flag insrc/frontend/src/lib/state/featureFlags.ts, off by default everywhere (includingid.ai/beta.id.ai— no domain override). Documented with the reason and the flip condition.AccessLevelToggleis hidden in all three flows (ContinueView,CliAuthorizeView,McpAuthorizeView) when the flag is off.effectiveAccessLevelthat is forced tofull-accesswhile the flag is off. This is the load-bearing part:/cliand/mcpdefault their (now-hidden) toggle to read-only, so simply hiding it would otherwise leave them minting the unusable queries-only delegations. With the flag off, every flow sends full access.toHaveCount(0)).The backend capability, the candid
Permissionstype, and all delegation tests from #4016 are unchanged and remain in place — re-enabling read-only is a one-line flip of the flag default once agents preserve the field.Tests
tsc --noEmit/svelte-check: 0 errors.eslint,prettier: clean.toPermissionsArgmapping and the/cli+/mcpauthorize-helper argument tests (unaffected — they exercise the mapping directly, below the flag).cli,mcp,authorize) updated to assert the access-level toggle is not rendered while the flag is off.🤖 Generated with Claude Code
https://claude.ai/code/session_018gHRjyFYqumMxAMwbPRLuN
Generated by Claude Code