feat: sign a canister call for submission from another machine - #716
Open
lwshang wants to merge 4 commits into
Open
feat: sign a canister call for submission from another machine#716lwshang wants to merge 4 commits into
lwshang wants to merge 4 commits into
Conversation
`icp canister call --sign-only <FILE>` composes and signs a call and writes it out instead of submitting it, so a machine that holds the key can prepare a call with no network at all and a machine with network can submit it without holding the key. `-` writes to stdout. Nothing in the signing path reaches the network: the Candid interface comes from `--candid` or the canister's local build artifact rather than from the canister itself, and a `fetch` root key is refused up front with a pointer at `--root-key mainnet` rather than left to time out. `--proxy` is rejected — the envelope would target the proxy and return a `ProxyResult` the submitting side would have to unwrap. `--valid-from <WHEN>` places the submission window, as a duration from now or an RFC 3339 timestamp. The window is always five minutes wide, because the IC rejects an ingress message whose expiry is further ahead than that, so the flag places the window rather than sizing it. This is deliberately not dfx's `--expire-after`, which names the window's *end* and leaves the user to subtract the five minutes. The file format lives in the `icp` crate as `signed_message`, since it is library surface rather than CLI plumbing, and `--sign-only` on other call-making commands can reuse it. Its `Destination` is tagged canister-or-subnet rather than a bare principal: the two route to different endpoints and the discriminant cannot be re-derived, and `ic-agent`'s `From<Principal> for EffectiveId` silently picks the canister endpoint. `validate()` re-derives everything from the envelope and refuses a file whose metadata disagrees. `Agent` creation grows an ingress-expiry override, used only for signing: `sign_request_status` derives its expiry from the agent and truncates the seconds, so without pinning it the pre-signed status check would land in a different five-minute window than the call it waits on. The chosen expiry is minute-aligned to make that truncation a no-op, and signing verifies the two agree before writing anything. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`RequestType` did not say whether it meant the called method's kind or
the API used to invoke it — a real ambiguity, since the two do not
correspond: an update method can only go through `/call`, a composite
query only through `/query`, and a query method through either.
It is the API, and the codebase already had a name for exactly that
distinction: `sync-plugin.wit`'s `enum call-type { update, query }`,
which dispatches `agent.update()` against `agent.query()` for the same
reason. Adopt it, and document what the type selects and why it cannot be
re-derived from the interface.
The wire format is unchanged: the field keeps `#[serde(rename = "type")]`
and the values stay `update` / `query`, as the design specifies and as
both the plugin ABI and quill spell them.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds offline signing for canister calls, producing portable signed-message JSON files for later submission.
Changes:
- Adds
--sign-onlyand--valid-fromcall options. - Introduces the signed-message format, validation, and tests.
- Supports offline interface and network configuration resolution.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
docs/reference/cli.md |
Documents signing options. |
crates/icp/src/signed_message.rs |
Implements the signed-message format and validation. |
crates/icp/src/network/access.rs |
Adapts agent creation. |
crates/icp/src/lib.rs |
Exports signed-message APIs. |
crates/icp/src/context/mod.rs |
Adds signing-specific agent construction. |
crates/icp/src/agent.rs |
Supports configurable ingress expiry. |
crates/icp/Cargo.toml |
Adds serialization dependencies. |
crates/icp-cli/tests/canister_call_sign_tests.rs |
Tests offline signing workflows. |
crates/icp-cli/src/commands/canister/call.rs |
Implements signing CLI behavior. |
crates/icp-cli/Cargo.toml |
Adds test serialization dependency. |
CHANGELOG.md |
Records the experimental feature. |
Cargo.toml |
Configures CBOR and time parsing. |
Cargo.lock |
Locks dependency changes. |
Suppressed comments (3)
crates/icp-cli/src/commands/canister/call.rs:456
- This always routes to
cid, but management-canister calls require an effective destination derived from their arguments (as already documented inoperations/proxy.rs:37-40). For example, a signedinstall_codecall will recordaaaaa-aainstead of the target canister, so the submitting side will use the wrong endpoint and the call cannot succeed. Resolve the effective destination once and use it both here and forsign_request_statusat line 424; reject methods whose route cannot be derived offline.
// Every call this command can compose is routed by canister id. A
// subnet-scoped destination is legal in the format, but nothing produces
// one yet.
destination: Destination::Canister(cid),
crates/icp-cli/src/commands/canister/call.rs:386
--valid-frompromises that the supplied instant is when the window opens, but flooring the expiry also floorsopens_at. Thus10:07:59Zopens at10:07:00Z, and a duration such as1scan produce a window that opened almost a minute before the command ran. That can make a scheduled call valid earlier than requested. Either reject inputs that are not minute-aligned or explicitly define and safely apply a rounding policy that never opens the window early.
let valid_until = floor_to_minute(opens_at + signed_message::SUBMISSION_WINDOW);
let valid_from = valid_until - signed_message::SUBMISSION_WINDOW;
crates/icp-cli/src/commands/canister/call.rs:474
- This validation uses the
nowcaptured before identity loading/signing, andvalidate()deliberately reportsWindowState::Expiredwithout returning an error. If signing is delayed (for example by a hardware token or process suspension), this can still write an already expired message. Validate against a fresh clock value and explicitly refuseExpiredbefore writing.
message
.validate(now)
.context("the signed message failed its own validation")?;
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
`--valid-from` accepts any duration that fits a `u64` of seconds, which reaches far past the last representable instant, and `OffsetDateTime`'s `+` panics rather than saturating — so `--valid-from 999999999999d` aborted the process. Place the window with checked arithmetic and report the input as out of range. Alongside, three things the same review surfaced: - Re-read the clock before the message validates itself, and refuse to write one whose window has already closed. Unlocking the identity and signing on a hardware token both take time, and a file that can no longer be submitted is worse than an error. - `--valid-from` promises the instant the window opens, but the expiry is floored to the minute to survive `ic-agent`'s own truncation, so the window can open up to 59 seconds early. Say so in the help text. - `summary.signed_at` is the one summary field the envelope cannot corroborate, since an ingress envelope carries no signing time. Document it as a note from the signer rather than leaving `Summary` claiming that all of it is re-derived and checked. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Restores what
dfx canister sign/dfx canister sendcovered: sign a call on a machine that holds the key and has no network, carry the file to a machine that has network and no key, submit it there.Sign side only.
icp message sendfollows in a second PR, so nothing here names it yet.A flag on
callrather than a new command: same inputs, stops one step earlier.-writes to stdout.Two things worth the reviewer's attention
Nothing in the signing path touches the network. Candid is not fetched (
--candid→ the local build artifact → none); the root key is recorded rather than resolved, so--root-key fetchis refused up front instead of timing out;--proxyis rejected; and the agent is built last, so no prompt can sit between resolving the network and signing.The window, and why the status check shares it. The IC accepts an ingress message only while its expiry is in the future and ≤5 min ahead of replica time, so the window is always
[expiry − 5min, expiry]— five minutes wide, always.--valid-fromnames its start (unlike dfx's--expire-after, which names the end and leaves you to subtract the width). The catch:Agent::sign_request_statusderives its expiry from the agent and truncates the seconds, which would put the pre-signed status check in a different window than the call it waits on. SoCreate::creategains an ingress-expiry override pinned from an absolute deadline measured after the identity is unlocked, the expiry is minute-aligned to make the truncation a no-op, and signing verifies the two agree before writing.Format
icp-signed-messagev1, in theicpcrate assigned_message— library surface, reusable by--sign-onlyon other commands. JSON, so a courier can read it; four objects mirroring the trust tiers (authenticated envelope / acted-upon-but-unauthenticated routing / display-only).destinationis tagged{"canister": …}/{"subnet": …}, not a bare principal: the two route to different endpoints and the discriminant can't be re-derived.ic-agent'sFrom<Principal> for EffectiveIdsilently picks the canister endpoint, so the type deliberately has no such impl.validate()re-derives everything from the envelope and refuses a file whose metadata disagrees. It reports the window state rather than enforcing it, so a reader can still show an expired message. Signing runs it on its own output — a mistake found on the other machine is found too late.Tests
15 unit tests in
icp::signed_message(round-trip, tampered summary/window, request-id mismatch, a status check signed for a different request, each window state, subnet-destination legality). 9 integration tests that start no network — the URLs are deliberately unreachable, since a test depending on a reachable one would stop testing the feature. The "both envelopes share a window" assertion decodes the two CBOR envelopes out of the written file.Known limits, all pre-existing in
callcanister callpasses no effective canister id toupdate_or_proxy_raw, so a management-canister call is routed byaaaaa-aaand misrouted — here as it already is online. Likewise nothing rejects acomposite_querysigned without--query. Both fixes belong to the online path too. A--candidfile usingimportembeds unresolved imports, degrading to the hex fallback on the submitting side.🤖 Generated with Claude Code