Skip to content

feat: make root key optional for connected networks#639

Draft
lwshang wants to merge 3 commits into
mainfrom
feat/optional-root-key-connected-networks
Draft

feat: make root key optional for connected networks#639
lwshang wants to merge 3 commits into
mainfrom
feat/optional-root-key-connected-networks

Conversation

@lwshang

@lwshang lwshang commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Makes the root key optional for connected networks, resolved by trust scope when the network is accessed:

Case Behavior
Explicit key (manifest root-key / --root-key) used as-is (any network)
Omitted + loopback URL fetched from the network's status endpoint
Omitted + remote URL hard error, fail fast

The built-in ic network is unchanged — still the hardcoded mainnet key, non-overridable.

Motivation

Previously --network <URL> required --root-key, and an omitted manifest key silently defaulted to the mainnet key. A local (loopback) network has no stable key to hardcode, so connecting to one by URL meant pasting a 266-char hex key.

  • Fetching on loopback is safe — there's no meaningful man-in-the-middle surface on localhost.
  • Refusing on remote surfaces a clear, actionable error at connection time (pass --root-key, or use --network ic for mainnet) instead of a cryptic certificate-verification failure on the first certified call. A genuine remote network never shares mainnet's key, so the old mainnet-default only deferred the failure — this is a diagnostics improvement, not a security change (both are fail-safe).

Implementation

  • Connected.root_key: Vec<u8>Option<Vec<u8>> (runtime type only; the manifest field was already optional). The manifest→runtime conversion no longer defaults to the mainnet key.
  • Resolution happens in one place — get_connected_network_accessresolve_root_key (crates/icp/src/network/access.rs) — which loopback-checks the URL, then fetches (anonymous agent → fetch_root_keyread_root_key) or fails with RootKeyRequiredForRemote. Runs before any RPC, so it covers both the CLI and manifest paths. NetworkAccess.root_key stays Vec<u8>, so identity/delegation and set_root_key downstream are untouched.
  • CLI: --network <URL> no longer forces --root-key; a named network still rejects it.

Notable consequence

Connecting to actual mainnet by raw URL without a key now errors instead of silently working — the message points users to --network ic or --root-key.

Testing

  • Added a no---root-key call against a running local network to canister_call_with_url_and_root_key (exercises the loopback-fetch path end-to-end).
  • Added canister_call_remote_url_without_root_key_errors (fast, no live server — the loopback check short-circuits before any request).
  • cargo fmt --check clean, cargo clippy zero warnings, full-workspace cargo test --no-run compiles.
  • Config schema regenerated → no diff. CLI docs (docs/reference/cli.md) regenerated.

🤖 Generated with Claude Code

lwshang and others added 3 commits July 6, 2026 14:58
The root key for a connected network is now optional and resolved by
trust scope when the network is accessed:

  - explicit key (manifest `root-key` / `--root-key`) -> used as-is
  - omitted + loopback URL -> fetched from the network status endpoint
  - omitted + remote URL   -> hard error, fail fast

Previously `--network <URL>` required `--root-key`, and an omitted
manifest key silently defaulted to the mainnet key. A local (loopback)
network never had a stable key to hardcode, so connecting by URL meant
pasting a 266-char key. Fetching on loopback is safe (no meaningful MITM
surface); refusing on remote surfaces a clear error early instead of a
cryptic certificate-verification failure on the first certified call. The
built-in `ic` network is unchanged: still the hardcoded mainnet key,
non-overridable.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`status_connected_network` defined a connected network at a remote URL
(https://ic0.app) with no root key and expected success. Under the new
contract a remote connected network must specify its root key, so the
manifest now includes the mainnet root key. Also adds a test asserting
that a keyless remote connected network fails fast with a clear message.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

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

This PR updates how connected networks handle root keys by making the runtime Connected.root_key optional and resolving it at access time: fetch automatically for loopback URLs, and fail fast with a clear error for remote URLs. This improves ergonomics for local networks while avoiding the previous (misleading) default-to-mainnet behavior.

Changes:

  • Changed runtime Connected.root_key from Vec<u8> to Option<Vec<u8>> and stopped defaulting missing manifest keys to the mainnet key.
  • Centralized root-key resolution in get_connected_network_access() via resolve_root_key() (loopback fetch vs remote error).
  • Updated CLI parsing/docs and added tests covering loopback fetch and remote-without-key failure.

Reviewed changes

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

Show a summary per file
File Description
docs/reference/cli.md Updates --root-key option documentation to reflect loopback-fetch vs remote-required behavior.
crates/icp/src/project.rs Updates default mainnet connected network construction to wrap the root key in Some(...).
crates/icp/src/network/mod.rs Makes Connected.root_key optional, adds serde helper for optional hex encoding, and removes mainnet defaulting during manifest conversion.
crates/icp/src/network/access.rs Resolves optional root keys at access time (fetch on loopback; error on remote) and introduces new error variants.
crates/icp/src/lib.rs Updates mock project loader’s connected mainnet network to use Some(IC_ROOT_KEY...).
crates/icp/src/context/mod.rs Makes URL-based network selection carry an optional root key and propagates it into the connected network config.
crates/icp-cli/tests/network_status_tests.rs Updates/extends status tests to require explicit key for remote connected networks and assert failure without it.
crates/icp-cli/tests/canister_call_root_key_tests.rs Adds end-to-end loopback fetch test and a remote URL missing-key error test.
crates/icp-cli/src/options.rs Makes --root-key optional for URL networks at CLI parse time and defers remote/loopback enforcement to access-time resolution.
CHANGELOG.md Documents the behavioral change, including the new error on raw mainnet-by-URL without a key.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants