Skip to content

test(execution): reject mgmt canister /call under queries-only delegation#10782

Merged
aterga merged 4 commits into
masterfrom
arshavir/queries-only-delegation-mgmt-canister
Jul 22, 2026
Merged

test(execution): reject mgmt canister /call under queries-only delegation#10782
aterga merged 4 commits into
masterfrom
arshavir/queries-only-delegation-mgmt-canister

Conversation

@aterga

@aterga aterga commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Adds a new queries_only_delegation_test system-test target in rs/tests/execution/ that verifies the queries-only delegation permissions feature end-to-end.
  • The test authenticates every request through a delegation chain whose top delegation carries permissions = queries, iterates over every variant of ic_management_canister_types_private::Method, and POSTs an ingress /call to aaaaa-aa for each method.
  • Each request must be rejected during ingress validation with 400 Bad Request and a body containing "Update calls are not permitted".

Why raw HTTP

ic-agent 0.47.3 (the version this workspace is currently on) does not expose the permissions field on Delegation; that landed in ic-agent 0.48.0 (crates.io release agent-rs#734). Once the workspace bumps to 0.48+, this test can migrate to ic-agent::identity::{Delegation, DelegatedIdentity} directly and drop the raw-HTTP scaffolding.

Test plan

  • bazel test //rs/tests/execution:queries_only_delegation_test passes on CI
  • Extending Method (e.g. a new IC00 endpoint) automatically exercises the new variant — the loop is driven by strum::IntoEnumIterator

🤖 Generated with Claude Code

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new execution system test that exercises the “queries-only delegation permissions” feature end-to-end by submitting /call requests to the management canister using a delegation chain restricted to permissions = queries, and asserting these update calls are rejected at ingress validation time.

Changes:

  • Introduces queries_only_delegation_test.rs, which iterates over all ic:00 methods and verifies /call is rejected with 400 Bad Request and an “Update calls are not permitted” message when authenticated via a queries-only delegation.
  • Registers a new Bazel system_test target //rs/tests/execution:queries_only_delegation_test with the required Rust dependencies.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
rs/tests/execution/queries_only_delegation_test.rs New system test that builds a queries-only delegation chain and verifies management-canister /call requests are rejected for every ic:00 method.
rs/tests/execution/BUILD.bazel Adds a new system_test target for the queries-only delegation execution test.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@aterga
aterga force-pushed the arshavir/queries-only-delegation-mgmt-canister branch from 8298761 to c7adbe6 Compare July 15, 2026 10:57
@aterga
aterga marked this pull request as ready for review July 15, 2026 11:55
@aterga
aterga requested a review from a team as a code owner July 15, 2026 11:55
@aterga
aterga requested a review from mraszyk July 15, 2026 11:55
Comment thread rs/tests/execution/queries_only_delegation_test.rs Outdated
aterga and others added 3 commits July 22, 2026 14:57
…tion

Adds a system test that authenticates with a delegation chain whose
`permissions` field is set to `queries`, then iterates over every
management canister method and POSTs an ingress `/call` to
`aaaaa-aa`. Every call must be rejected with `400 Bad Request` and a
body mentioning that update calls are not permitted.

The test is a self-contained raw-HTTP client because `ic-agent 0.47.3`
does not expose the `permissions` field on `Delegation`; only the
newer 0.48.x line does. Once the workspace bumps to 0.48+ this can
migrate to `ic-agent::Delegation`/`DelegatedIdentity` directly.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The previous substring "Update calls are not permitted" is the prefix
of the validator error; matching only it would also accept any future
rejection whose message happens to start that way. Match the tail
that identifies the queries-only delegation restriction explicitly:
`a delegation restricts the sender to query calls
(permissions = "queries")`. This anchors the test to the exact
`RequestValidationError::UpdateCallNotPermittedByDelegation` message
(see `rs/validator/ingress_message/src/lib.rs`).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Workspace is now on `ic-agent 0.49.0`, which exposes the `permissions`
field on `Delegation` and `DelegationPermissions::{Queries, All}` under
`ic_agent::identity`. Rebuild the test on top of the standard agent:

* construct the `Delegation` (with `permissions = Some(Queries)`)
  directly using `ic-agent`'s public types;
* sign it with the sender's `Identity::sign_arbitrary`;
* wrap `sender_pubkey` + inner session identity + signed delegation in
  `DelegatedIdentity::new_unchecked` and hand it to `agent_with_identity`
  (which internally calls `fetch_root_key`);
* dispatch each `Ic00Method` through `agent.update(...).call_and_wait()`
  and assert the returned `AgentError::HttpError` carries a `400` and
  a body matching the queries-only-restriction reason.

Drops the manual `HttpRequestEnvelope` / `SignedDelegation` construction
and the reqwest / serde_cbor deps.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aterga
aterga force-pushed the arshavir/queries-only-delegation-mgmt-canister branch from e33d8d5 to 96cf4ff Compare July 22, 2026 13:05
Addresses PR feedback: the queries-only delegation restriction applies
to every `/call` endpoint, not just the management canister. Split the
test into two `pub fn`s sharing the same `SystemTestGroup`:

* `management_canister_calls_rejected_with_queries_only_delegation`
  keeps the existing per-`Ic00Method` sweep.
* `regular_canister_call_rejected_with_queries_only_delegation`
  installs a UC (via the default agent — the queries-only agent cannot
  install code) and asserts that an ordinary `update` call to it is
  rejected with the same `400 Bad Request` and body.

The delegation-building logic and the rejection assertion are factored
into `build_queries_only_agent` and `assert_update_rejected`
respectively.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aterga
aterga added this pull request to the merge queue Jul 22, 2026
Merged via the queue into master with commit 431c0cc Jul 22, 2026
37 checks passed
@aterga
aterga deleted the arshavir/queries-only-delegation-mgmt-canister branch July 22, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants