Skip to content

docs(security): port security best practices 1:1 from portal#204

Merged
marc0olo merged 23 commits into
mainfrom
docs/security-port
May 12, 2026
Merged

docs(security): port security best practices 1:1 from portal#204
marc0olo merged 23 commits into
mainfrom
docs/security-port

Conversation

@marc0olo
Copy link
Copy Markdown
Member

@marc0olo marc0olo commented May 5, 2026

Summary

  • Replaces AI-generated security docs with verified portal content to eliminate correctness bugs (confirmed double-spend bug in inter-canister-calls.md)
  • Ports all security section files from dfinity/portal (building-apps/security/) 1:1 as the content base
  • Adds 7 missing topic files: overview.md, data-storage.md, decentralization.md, https-outcalls.md, miscellaneous.md, observability-and-monitoring.md, formal-verification.md
  • Merges resources.md content into overview.md as a "Further reading" section (no value as a standalone thin page)
  • Renames 4 slugs for SEO clarity:
    • access-management.mdxidentity-and-access-management.mdx
    • data-integrity.mddata-integrity-and-authenticity.md
    • observability.mdobservability-and-monitoring.md
    • misc.mdmiscellaneous.md
  • Removes encryption.mdx (AI-generated, unreviewed; vetKeys encryption guide will be written from scratch separately)
  • Adds 2 prerequisite reference pages: references/message-execution-properties.md and guides/canister-calls/idempotency.md
  • Replaces retry_idempotency.png image with a PlantUML sequence diagram in idempotency.md
  • Reorders security sidebar by learning progression (overview → IAM → storage → integrity → inter-canister → HTTPS → DoS → upgrades → observability → decentralization → misc → formal verification)
  • Updates mo:base/HashMap CallerGuard in inter-canister-calls.md to mo:core/Map (only code change beyond 1:1 port)
  • Fixes all CI validation errors: Upstream comment format, em-dashes in prose, broken internal links

Notes

  • @dfinity/agent references in the ported files are left as-is; updating to the new JS SDK is a separate follow-up
  • The confirmed double-spend bug was in inter-canister-calls.md: it suggested issuing a refund after receiving a bounded_wait error, where the transfer could still have gone through
  • concepts/security.md (new architectural overview, not from portal) is kept as-is; flagged for separate security team review

Sync recommendation

sync from dfinity/portal building-apps/security/

Tracked in: #203

@marc0olo
Copy link
Copy Markdown
Member Author

marc0olo commented May 5, 2026

Plan for security team review

Hi @dfinity/product-security — we'd like your input on this PR before it merges. Here's the context and what we're asking.

What this PR does

This is a 1:1 port of all security best practices from dfinity/portal (building-apps/security/) to this site. The primary goal was to replace AI-generated rewrites that had diverged from the carefully reviewed portal content, including a confirmed correctness bug (double-spend scenario in inter-canister-calls.md).

The only intentional deviations from a pure port are:

  • 4 slug renames for SEO clarity (access-managementidentity-and-access-management, miscmiscellaneous, etc.) — old URLs were broken anyway since the section was new
  • resources.md merged into overview.md as a "Further reading" section (thin standalone page, no content lost)
  • encryption.mdx removed — it was AI-generated and had never been security-reviewed; a proper vetKeys encryption guide will be written separately from the vetkd icskill
  • Motoko CallerGuard updated from mo:base/HashMap to mo:core/Map (library migration required by project rules, not a security change)

You are now added as CODEOWNER for all files in docs/guides/security/, docs/concepts/security.md, docs/references/message-execution-properties.md, and docs/guides/canister-calls/idempotency.md. Your approval is required for any future changes to these files.

What we're asking from you

Please verify the ported content is correct and matches the portal source. We're keeping this PR in draft intentionally — it will not be merged until after a second cleanup PR (described below) is also approved.

What comes next — cleanup PR

After your approval of this PR, we'll create a second branch from docs/security-port to address editorial issues that go beyond the 1:1 port. We'd need your approval on that one too before anything merges to main. Known items for that cleanup:

  1. Deprecated example link: decentralization.md links to https://github.com/dfinity/examples/tree/master/rust/basic_dao — this example no longer exists in the examples repo.

  2. Outdated JS SDK references: data-integrity-and-authenticity.md imports from @dfinity/agent and identity-and-access-management.mdx links to agent-js — these should be updated to the current @icp-sdk/* packages.

  3. Brand guideline violations: "dapps", "smart contracts", ... appear throughout, particularly in decentralization.md and overview.md. We'll align with current terminology in the cleanup — though we'd appreciate your input on whether any of these are load-bearing technical terms in the security context that should be preserved.

  4. ICP Wiki links: 4 files link to wiki.internetcomputer.org, which is unmaintained and no longer the source of truth:

    • data-storage.md, canister-upgrades.md, inter-canister-calls.md (×2) — all pointing to "Current limitations of the Internet Computer" (covering pre_upgrade bugs, malicious callees preventing upgrades, loops in call graphs)
    • decentralization.md — "Verification and trust in a (launched) SNS" and "SNS decentralization swap trust"

    These will need to be replaced with links to current docs or the content inlined directly. Security team input welcome on where the canonical replacement should be, particularly for the "current limitations" page which is referenced 3 times.

Merge sequence

  1. Your approval of PR docs(security): port security best practices 1:1 from portal #204 (this PR) → stays in draft
  2. Cleanup branch → PR targets docs/security-port → your approval → merge into docs/security-port
  3. Un-draft and merge PR docs(security): port security best practices 1:1 from portal #204 into main

@marc0olo marc0olo force-pushed the docs/security-port branch from 51de28a to 14e63c9 Compare May 7, 2026 14:17
@marc0olo marc0olo marked this pull request as ready for review May 11, 2026 17:08
@marc0olo marc0olo requested a review from a team as a code owner May 11, 2026 17:08
marc0olo added a commit that referenced this pull request May 12, 2026
…ces (#239)

## Summary
Closes #235. Post-merge cleanup for PR #204 after PR #208 landed.

- **`canister-control.md`**: SNS link →
`docs/concepts/governance.md#the-service-nervous-system`;
tokenomics/voting-power link → `docs/concepts/governance.md#neurons`;
removed "See also" wiki bullet (no internal equivalent for SNS
verification trust or swap trust content)
- **`canister-upgrades.md`**: Removed wiki "current limitations" bullet
for `pre_upgrade` bugs (no internal equivalent)
- **`data-storage.md`**: Removed wiki "current limitations" bullet for
long running upgrades and deserializer memory (no internal equivalent)
- **`inter-canister-calls.md`**: Removed two wiki "current limitations"
bullets for untrustworthy canisters and call graph loops (no internal
equivalent)
- **`data-integrity-and-authenticity.md`**: Asset certification Learn
Hub link → `docs/guides/frontends/certification.md`

Note: the rebase of `docs/security-port` on `main` is deferred — will be
done as a final step before that PR merges.

## Sync recommendation
hand-written (link fixes only; no content changes)
marc0olo and others added 20 commits May 12, 2026 09:14
Replaces AI-generated security docs with verified portal content,
adds 8 missing topic files, 2 new prerequisite pages, and fixes
a confirmed double-spend bug in the inter-canister-calls guide.

Changes:
- Replace: inter-canister-calls.md, access-management.mdx,
  canister-upgrades.md, dos-prevention.md, data-integrity.md
- Add: overview.md, data-storage.md, decentralization.md,
  https-outcalls.md, misc.md, observability.md, resources.md,
  formal-verification.md
- Add: references/message-execution-properties.md (prerequisite
  referenced by inter-canister-calls.md)
- Add: guides/canister-calls/idempotency.md (prerequisite for
  safe retry patterns in inter-canister calls)
- Fix sidebar order conflicts (now matches portal ordering 1-14)
- Fix MDX HTML comment syntax in access-management.mdx
- Add security and reference diagram images to public/img/
- encryption.mdx flagged for separate security team review (new
  content not from portal, not changed here)
## Summary

- **\"dapp\"/\"dapps\" → \"app\"/\"apps\"** across all 12 security guide
files; repository names in URLs preserved (`nns-dapp`,
`encrypted-notes-dapp`), link labels updated (`NNS app`, `encrypted
notes`)
- **\"smart contract(s)\" → \"canister(s)\"** in `decentralization.md`,
including the section heading and the blockchains admonition note
- **Em-dashes removed from `<!-- Upstream: -->` comments** in all 11
remaining files (`identity-and-access-management.mdx` was already fixed
in a previous commit)
- **Informal phrasing removed** in `data-integrity-and-authenticity.md`:
\"we will club composite_query\" and \"best of both worlds\"
- **Garbled sentence fixed** in `identity-and-access-management.mdx`
(mobile II section): the original had a sentence fragment mid-paragraph
from a copy-paste error
- **\"DAO\" removed from prose** in `decentralization.md`; replaced with
\"community governance\", \"governance framework\", and \"custom
governance canister\" following the convention established in PR #208
- **\"decentralized governance system\" → \"governance framework\"**
throughout `decentralization.md`
- **`composite_query` description corrected** in
`data-integrity-and-authenticity.md`: \"query call\" → \"query methods\"
(composite_query is a method type, not a call type)
- **\"off-chain\" → \"offchain\" / \"external\"** in
`decentralization.md`; bare \"onchain\"/\"offchain\" category labels
replaced with descriptive terms (\"external components\", \"hosted as
canisters\")
- **\"tamper-resistant\" → \"tamperproof\"** in
`observability-and-monitoring.md` (one word, per brand guide)
- **\"on-chain\" → \"stored in the canister\"** in
`observability-and-monitoring.md`

## Sync recommendation

`informed by dfinity/portal` — content is derived from the portal source
but diverges intentionally for brand voice compliance; no sync back to
portal is expected.
… usage (#236)

## Summary

- **overview.md**: Update 3 ANSSI Rust guide links to the restructured
URL paths (`introduction.html`, `unsafe/generalities.html`,
`integer.html#chapter-integer`, `libraries.html#cargo-audit`)
- **data-storage.md**: Remove abandoned `seniorjoinu/ic-stable-memory`
library (unmaintained since May 2023) and its caution block; rephrase
intro to single remaining library; update encrypted notes example link
from defunct `motoko/encrypted-notes-dapp` to
`rust/vetkeys/encrypted_notes_dapp_vetkd`
- **decentralization.md**: Remove LaunchTrail reference
(`spinner-cash/launchtrail`, abandoned June 2022); remove `basic_dao`
example link (path no longer exists on master)
- **canister-upgrades.md**: Update both `set_timer_interval` links from
`ic-cdk/0.6.9` to `ic-cdk-timers/1.0.0` (function moved crates)
- **data-integrity-and-authenticity.md**: Migrate JavaScript client-side
verification example from deprecated `@dfinity/agent`,
`@dfinity/candid`, `@dfinity/principal` to `@icp-sdk/core/agent`,
`@icp-sdk/core/candid`, `@icp-sdk/core/principal`; update `HttpAgent`,
`Certificate`, and `lookup_path` APIs to v5; fix pre-existing
`start().await` bug

## Sync recommendation

`sync from dfinity/portal building-apps/security/*` — changes are fixes
on top of the ported content; upstream source does not yet reflect the
updated SDK or fixed links.
- Replace both mermaid sequence diagrams with plantuml equivalents
  using the already-configured remark-plantuml plugin
- Fix unity_ii_deeplink example links: main -> master branch
…ter-control (#237)

## Summary
- Apply sentence case to all security guide page titles (matches portal
convention and brand rules)
- Add `sidebar.label: "Overview"` to overview page so navbar shows
"Overview" while page title remains "Security overview"
- Rename `decentralization.md` → `canister-control.md` (more accurate:
covers SNS governance, canister trust verification, and untrusted asset
loading)
- Remove "Security" from individual page titles within the security
section (the section heading already provides context, consistent with
original portal structure)
- Improve three descriptions: "endpoint verification" (was
"validation"), "timer reinstatement after upgrades" (was
"reinstatement"), added "mobile Internet Identity integration" to IAM
description

## Sync recommendation
hand-written (title/description metadata changes only; content
unchanged)
…ces (#239)

## Summary
Closes #235. Post-merge cleanup for PR #204 after PR #208 landed.

- **`canister-control.md`**: SNS link →
`docs/concepts/governance.md#the-service-nervous-system`;
tokenomics/voting-power link → `docs/concepts/governance.md#neurons`;
removed "See also" wiki bullet (no internal equivalent for SNS
verification trust or swap trust content)
- **`canister-upgrades.md`**: Removed wiki "current limitations" bullet
for `pre_upgrade` bugs (no internal equivalent)
- **`data-storage.md`**: Removed wiki "current limitations" bullet for
long running upgrades and deserializer memory (no internal equivalent)
- **`inter-canister-calls.md`**: Removed two wiki "current limitations"
bullets for untrustworthy canisters and call graph loops (no internal
equivalent)
- **`data-integrity-and-authenticity.md`**: Asset certification Learn
Hub link → `docs/guides/frontends/certification.md`

Note: the rebase of `docs/security-port` on `main` is deferred — will be
done as a final step before that PR merges.

## Sync recommendation
hand-written (link fixes only; no content changes)
@marc0olo marc0olo force-pushed the docs/security-port branch from 40d8bf8 to a56e668 Compare May 12, 2026 07:15
raymondk
raymondk previously approved these changes May 12, 2026
@marc0olo marc0olo merged commit f71943e into main May 12, 2026
8 of 9 checks passed
@marc0olo marc0olo deleted the docs/security-port branch May 12, 2026 13:41
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