Skip to content

docs: correct the certification guides against current libraries - #408

Draft
marc0olo wants to merge 6 commits into
mainfrom
docs/certification-accuracy
Draft

marc0olo wants to merge 6 commits into
mainfrom
docs/certification-accuracy

Conversation

@marc0olo

@marc0olo marc0olo commented Sep 25, 2026 •

Copy link
Copy Markdown
Member

Summary

Both certification guides had claims that are wrong on current releases. Every claim was checked against primary sources (the crates, @icp-sdk/core 6.1.0, ic-gateway, the interface spec, icp-cli v1.5.0 cli.md), and every code block was compiled and run against a local network. The skill side of the same fixes is dfinity/icskills#407.

Wrong, now fixed:

  • "Certified data is cleared on upgrade" (5 places): it survives upgrades (abstract-behavior.md, confirmed locally). What is lost is a heap tree, so Rust rebuilds it in post_upgrade, and Motoko's CertTree.Store needs no hook.
  • Header name: IC-Certificate-Expression is really IC-CertificateExpression (gateway spec, crate constant, served headers).
  • icp canister call … get without --query: icp-cli sends an update call by default, so no certificate comes back.
  • Client verification code: it did not type-check against @dfinity/certificate-verification 4 (Uint8Array, not ArrayBuffer). lookupResultToBuffer treated Unknown as absent; it now switches on the lookup_path status.
  • The "custom HTTP client" case needs @dfinity/response-verification, not certificate-verification. This is now a "who verifies what" table (HTTP gateway, update calls, Candid queries, raw hosts).
  • The ic-asset-certification example did not compile (missing candid, 404 needs StatusCode). Its uncertified 404 was rejected by the gateway; it now uses a certified 404.html fallback.
  • Rust example: it lacked export_candid!(), so icp deploy failed. It now uses ic-certification 4 / ic-cdk 0.20.
  • The single-value Motoko example never certified its initial value, so a query before the first write failed verification (certified data starts empty).
  • Motoko CertTree example did not compile (CertTree.Ops must be transient). The deprecated postupgrade hook is removed, and remove becomes delete to match the test commands.
  • allow_raw_access: false: redirects with 308, but raw.icp.net lands on <id>.icp0.io, and only mainnet raw hosts are recognized.
  • Asset canister certified headers: also Cache-Control (with max_age) and Content-Encoding.
  • Intro: it linked "boundary nodes" to the API boundary nodes, which verify nothing.
  • Root key: shouldFetchRootKey is replaced by the ic_env cookie / icp network status --json.
  • Links: the js.icp.build link is dropped (that site does not document this package), and certified-counter (dfx, fetchRootKey) is replaced by motoko/cert-var.

After review:

Structural note: the asset and Rust examples stay inline even though they exceed the 30-line guideline, as before. They could move to dfinity/examples with #region markers.

Fixes claims that were wrong or outdated: certified data survives upgrades,
the certification header is IC-CertificateExpression, icp canister call needs
--query to return a certificate, and raw-access redirects land on icp0.io.
Code examples now compile and verify against ic-cdk 0.20, the 4.x
certification crates and @dfinity/certificate-verification 4.
@github-actions

github-actions Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

🤖 Here's your preview: https://keluh-vqaaa-aaaam-ai7wa-cai.icp0.io

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.

Copilot review overview

🟡 Changes recommended

The raw-host verification guidance and Rust upgrade example remain technically incorrect.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 4 Medium severity · 1 Low severity

Open (5)
What changed in this PR

Updates certification guides to match current ICP libraries and runtime behavior.

Changes:

  • Corrects HTTP certification headers, gateway behavior, and client verification.
  • Updates Rust, Motoko, TypeScript, and icp-cli examples.
  • Clarifies certified-data behavior across upgrades.
File Description
docs/​guides/​frontends/​certification.md Updates HTTP certification and verification guidance.
docs/​guides/​backends/​certified-variables.md Revises implementations, dependencies, and upgrade behavior.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/guides/backends/certified-variables.md Outdated
Comment thread docs/guides/backends/certified-variables.md Outdated
Comment thread docs/guides/frontends/certification.md Outdated
Comment thread docs/guides/frontends/certification.md
Comment thread docs/guides/frontends/certification.md Outdated
Scope the gateway guarantee to verifying hostnames, describe what each
certification header carries, name every context certified_data_set
allows, and state that the Rust example keeps its tree on the heap only.
@marc0olo

Copy link
Copy Markdown
Member Author

Feedback addressed:

  • The Rust post_upgrade comment now states that the example keeps its tree on the heap only.
  • certified_data_set contexts now match the interface spec (every replicated context; traps in a query).
  • The gateway guarantee is scoped to verifying hostnames.
  • The two certification headers are described accurately.
  • The raw-host wording is now "forwards it without checking it". Not changed: the verifyRequestResponsePair row, because raw responses keep their certificate (verified on mainnet and in the gateway source); upstream fix in docs: "on raw the gateway simply discards it" reads as if the certificate is removed certified-assets#138.

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.

Copilot review overview

🟡 Changes recommended

The replicated-context claim and raw-host root-key guidance remain inaccurate or unsafe.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity · 1 Medium severity

Open (2)
Resolved since last review (5)

Comment thread docs/guides/frontends/certification.md Outdated
Comment thread docs/guides/backends/certified-variables.md Outdated
… root key

certified_data_set is allowed in init, upgrade hooks, updates, reply and reject
callbacks and system tasks, not in cleanup callbacks or any query. The ic_env
root key is only trustworthy on a verifying hostname.
@marc0olo

Copy link
Copy Markdown
Member Author

Feedback addressed:

  • certified_data_set now lists the spec's allowed contexts explicitly, including pre_upgrade and the low-Wasm-memory hook, and excludes cleanup callbacks and queries.
  • The ic_env root key is scoped to verifying hostnames; a client verifying raw responses needs an independently obtained key.

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.

Copilot review overview

🔵 Needs a closer look

The single-value verifier does not accept the optional certificate type returned by its Motoko getter.

Review effort: Balanced
Findings: None

Resolved since last review (2)
Previously missed (1)

In code that hasn't changed since last review

Medium severity Handle optional certificate before Certificate.create

docs/​guides/​backends/​certified-variables.md:331

This helper is presented as the verifier for the Motoko getter above, but that getter returns certificate : ?Blob. Its generated response therefore has an optional certificate, so it cannot be passed to this { certificate: Uint8Array } parameter without an unchecked unwrap. Accept the nullable field and fail explicitly when the getter was not executed as a query before calling Certificate.create.

@marc0olo

Copy link
Copy Markdown
Member Author

Feedback addressed:

  • Both verification helpers accept the ?blob certificate that the Motoko getters return (Uint8Array | null), and fail with a clear message when it is null (the getter was not called as a query). Verified against live Rust and Motoko canisters, including the null case.

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.

Copilot review overview

🔵 Needs a closer look

The TypeScript helper does not accept the Motoko example’s Uint8Array value without normalization.

Review effort: Balanced
Findings: None

Previously missed (2)

In code that hasn't changed since last review

Medium severity CertTree blob getter type mismatches helper input

docs/​guides/​backends/​certified-variables.md:285

This helper is presented after both implementations, but the Motoko CertTree getter returns value : ?Blob, which bindgen exposes as Uint8Array | null; the declared string | null input therefore does not type-check with that example. Normalize the blob value before comparing it, or explicitly scope the helper to the Rust getter.

Low severity Clarify Motoko response values require blob decoding

docs/​guides/​frontends/​certification.md:274

“The response can be passed as is” is not true for the Motoko CertTree response linked from this section: its value : ?Blob becomes Uint8Array | null, while the helper accepts string | null. Clarify that only the certificate and witness already match, and that blob values must be decoded first.

@marc0olo

Copy link
Copy Markdown
Member Author

Feedback addressed:

  • The witness helper accepts both getters' responses as returned: the Rust opt text value and the Motoko CertTree ?Blob value (Uint8Array | null, decoded as UTF-8). The frontend guide no longer claims the response passes "as is" without saying which fields.
  • Verified with the raw responses of both live canisters, including an absent key and a tampered value.

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.

Copilot review overview

🟡 Changes recommended

The single-value verifier lacks the documented certificate freshness check, permitting stale replay.

Get a fresh assessment by requesting another Copilot review.

Review effort: Balanced
Findings: 1 High severity

Open (1)

Comment thread docs/guides/backends/certified-variables.md

This branch has not been deployed

No deployments
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