feat: new canister setting status_visibility#10667
Conversation
# Conflicts: # rs/execution_environment/tests/execution_test.rs # rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs
…n_test.rs Commit 2c0413f ("simplify") deliberately removed the three canister_status_via_query_call_* tests and the canister_status_count helper from execution_test.rs, as they are fully covered by the table-driven test_status_visibility_of_canister_status in canister_status.rs (which exercises every visibility x sender combination over both the query and update call paths). The merge of master (6ab53de) accidentally resurrected these tests during conflict resolution, reintroducing the stale negative test that still asserted the pre-status-visibility error code (CanisterInvalidControllerOrSubnetAdmin instead of the new CanisterStatusAccessDenied). Re-remove them to restore the intended state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bef2cc0 to
7849177
Compare
Re-add the query success-metric coverage lost when the canister_status_via_query_call_* tests were removed from execution_test.rs, folding it into the status_visibility matrix test: the successful canister_status query metric must increment exactly once for an allowed query call and never for a denied query or an update call. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Create the test canister with zero cycles: the subnet uses a free cost schedule, so no cycles are needed (removes the INITIAL_CYCLES_BALANCE constant that was carried over from the original tests). - Fix the two `.expect` messages on the success path so each matches the layer it unwraps: the outer Result is the access-control gate, the inner Result is the reply/reject layer (unreachable via ingress/query). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds a new canister setting, status_visibility, to control who can call the management canister’s canister_status endpoint, aligning it with existing visibility patterns (logs/snapshots) while preserving default behavior (controllers). The change propagates through the public management canister types, replicated state persistence (protobuf/checkpoints), access-control enforcement in the execution environment, and test coverage (including a new integration test matrix and updated error-code expectations).
Changes:
- Introduce
StatusVisibilityas a first-class setting in management types and canister settings APIs, and persist it in checkpoints/protobuf (tag 69). - Enforce
status_visibilityforcanister_statusacross ingress acceptance and execution paths, returning the newCanisterStatusAccessDeniederror code on denial. - Add/adjust tests to cover the access matrix and metric behavior; update existing tests for the new error code.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| rs/types/management_canister_types/tests/ic.did | Extends candid test interface with status_visibility and new settings fields. |
| rs/types/management_canister_types/src/lib.rs | Adds StatusVisibility type + protobuf conversions; wires it into settings/status result types. |
| rs/state_manager/src/tip.rs | Serializes status_visibility into checkpointed canister state bits. |
| rs/state_manager/src/checkpoint.rs | Loads status_visibility from persisted canister state bits. |
| rs/state_layout/src/state_layout/tests.rs | Updates default CanisterStateBits test fixture to include status_visibility. |
| rs/state_layout/src/state_layout/proto.rs | Adds protobuf encode/decode plumbing for status_visibility in CanisterStateBits. |
| rs/state_layout/src/state_layout.rs | Extends CanisterStateBits struct with status_visibility. |
| rs/rust_canisters/tests/test/canister_management.rs | Updates expected error code for denied canister_status. |
| rs/replicated_state/src/canister_state/system_state.rs | Adds status_visibility to SystemState and ensures default/init/checkpoint paths carry it. |
| rs/replicated_state/src/canister_state.rs | Exposes status_visibility() accessor on CanisterState. |
| rs/replica_tests/tests/canister_lifecycle.rs | Updates test construction of CanisterStatusResultV2 to include status_visibility. |
| rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs | Regenerates prost types to include StatusVisibility and tag 69 in CanisterStateBits. |
| rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto | Adds StatusVisibility messages and field 69; bumps Next ID. |
| rs/pocket_ic_server/src/pocket_ic.rs | Sets explicit default status_visibility when creating canisters in PocketIC scenarios. |
| rs/nervous_system/clients/src/update_settings.rs | Adapts client-side update_settings conversion to new field (currently always sending None). |
| rs/execution_environment/tests/execution_test.rs | Removes older canister_status query metric/access tests (replaced by new matrix test). |
| rs/execution_environment/tests/canister_status.rs | Adds new integration test covering {visibility} x {caller} x {update/query} plus metrics assertions. |
| rs/execution_environment/src/execution/common.rs | Introduces validate_status_visibility helper to centralize access checks. |
| rs/execution_environment/src/execution_environment/tests.rs | Updates ingress acceptance tests to expect CanisterStatusAccessDenied. |
| rs/execution_environment/src/canister_settings.rs | Extends settings parsing/builders and visibility abstraction to include StatusVisibility. |
| rs/execution_environment/src/canister_manager/types.rs | Adds CanisterStatusAccessDenied variant + mapping to UserError (error code 542). |
| rs/execution_environment/src/canister_manager/tests.rs | Updates manager tests to assert new error code/message for denied status calls. |
| rs/execution_environment/src/canister_manager.rs | Enforces status_visibility for canister_status (ingress acceptance + execution paths) and returns it in results. |
| rs/cycles_account_manager/src/cycles_account_manager.rs | Bumps max “delayed ingress induction cost” payload size constant for update_settings payload changes. |
Comments suppressed due to low confidence (1)
rs/nervous_system/clients/src/update_settings.rs:96
CanisterSettingsis documented as matching the ic-interface-spec, but the newstatus_visibilitysetting is not represented here and is always sent asNonein theCanisterSettingsArgsconversion. This means nervous-system clients cannot setstatus_visibility(even to the defaults likecontrollers/public), and the local struct is now out of sync with the management canister interface.
management_canister::CanisterSettingsArgs {
controllers: controllers.map(management_canister::BoundedControllers::new),
compute_allocation,
memory_allocation,
freezing_threshold,
reserved_cycles_limit,
log_visibility: log_visibility.map(management_canister::LogVisibilityV2::from),
snapshot_visibility: snapshot_visibility
.map(management_canister::SnapshotVisibility::from),
status_visibility: None,
log_memory_limit: None,
wasm_memory_limit,
wasm_memory_threshold,
environment_variables: None,
minimum_incoming_canister_call_cycles: None,
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
This pull request changes code owned by the Governance team. Therefore, make sure that
you have considered the following (for Governance-owned code):
-
Update
unreleased_changelog.md(if there are behavior changes, even if they are
non-breaking). -
Are there BREAKING changes?
-
Is a data migration needed?
-
Security review?
How to Satisfy This Automatic Review
-
Go to the bottom of the pull request page.
-
Look for where it says this bot is requesting changes.
-
Click the three dots to the right.
-
Select "Dismiss review".
-
In the text entry box, respond to each of the numbered items in the previous
section, declare one of the following:
-
Done.
-
$REASON_WHY_NO_NEED. E.g. for
unreleased_changelog.md, "No
canister behavior changes.", or for item 2, "Existing APIs
behave as before.".
Brief Guide to "Externally Visible" Changes
"Externally visible behavior change" is very often due to some NEW canister API.
Changes to EXISTING APIs are more likely to be "breaking".
If these changes are breaking, make sure that clients know how to migrate, how to
maintain their continuity of operations.
If your changes are behind a feature flag, then, do NOT add entrie(s) to
unreleased_changelog.md in this PR! But rather, add entrie(s) later, in the PR
that enables these changes in production.
Reference(s)
For a more comprehensive checklist, see here.
GOVERNANCE_CHECKLIST_REMINDER_DEDUP
|
✅ No security or compliance issues detected. Reviewed everything up to f092104. Security Overview
Detected Code Changes
|
|
✅ No security or compliance issues detected. Reviewed everything up to f092104. Security Overview
Detected Code Changes
|
No canister behavior changes.
What
Introduces a new canister setting,
status_visibility, that governs whichprincipals are allowed to read a canister's status via the
canister_statusmanagement endpoint. It mirrors the existing
log_visibilityandsnapshot_visibilitysettings:controllers(default): only the canister's controllers (and subnetadmins) can read the status — the pre-existing behavior.
public: anyone can read the status.allowed_viewers: the controllers, the subnet admins, and up to 10explicitly listed principals can read the status.
How
validate_status_visibilityhelper reuses theexisting
VisibilitySettings::has_accessmechanism. It is enforced on boththe ingress-acceptance path (
should_accept_ingress_message) and bothexecution paths (replicated update and non-replicated query) via
get_canister_status. A canister reading its own status is still exempt.status_visibilityis added toSystemState, persisted inCanisterStateBits(proto tag 69). Checkpoints written before this changedecode to the default (
controllers), so loading old state isbackward-compatible.
canister_statuscalls now return the dedicatedCanisterStatusAccessDenied(542) error code, replacing the previousCanisterInvalidController/CanisterInvalidControllerOrSubnetAdmin. It mapsto the same
CanisterErrorreject code as before.Compatibility notes
the setting default to
controllers, so access is unchanged.ErrorCodefor adenied
canister_status(previouslyCanisterInvalidController/CanisterInvalidControllerOrSubnetAdmin) will now seeCanisterStatusAccessDenied. The reject code is unchanged.Testing
rs/execution_environment/tests/canister_status.rsexercises the full matrix of
{status_visibility} x {controller, subnet_admin, allowed_viewer, other} x {update, query}call paths, asserting thegranted/denied outcome (and that canister_status returns the expected
status visibility upon success), and, on denial, the returned error code and
description.
canister_statusquery success metric incrementsexactly once for an allowed query call and never for a denied query or an
update call — restoring (and broadening) the coverage from the
canister_status_via_query_call_*tests removed fromexecution_test.rs.