Skip to content

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

Draft
marc0olo wants to merge 12 commits intomainfrom
docs/security-port
Draft

docs(security): port security best practices 1:1 from portal#204
marc0olo wants to merge 12 commits intomainfrom
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 added 12 commits May 5, 2026 21:32
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)
@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. Broken external link: https://www.joachim-breitner.de/blog/788-How_to_audit_an_Internet_Computer_canister — site is down (ECONNREFUSED). Appears 9 times across 6 files. We need to decide: find a replacement URL, point to a web archive, or remove the references.

  2. 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.

  3. 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.

  4. 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.

  5. 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

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.

1 participant