Skip to content

Security review and changes for multi-canister skill#57

Merged
robin-kunzler merged 4 commits intomainfrom
robin-kunzler/sec-review-multi-canister-skill
Mar 3, 2026
Merged

Security review and changes for multi-canister skill#57
robin-kunzler merged 4 commits intomainfrom
robin-kunzler/sec-review-multi-canister-skill

Conversation

@robin-kunzler
Copy link
Copy Markdown
Contributor

@robin-kunzler robin-kunzler commented Mar 2, 2026

  • Restructured "mistakes that break your build" into three sections: funcional issues, security issues, build issues. I think the first two are not really about breaking the build so I think this is clearer.
  • Added several issues and links to security issues
  • Reviewed the code examples, did minor fixes directly. Instead of hardening the code security directly, I added sections about what would be needed to make them production ready. I think that's better as it keeps the examples focused without cluttering them.

@robin-kunzler robin-kunzler marked this pull request as ready for review March 2, 2026 13:49
Copy link
Copy Markdown
Contributor

@alin-at-dfinity alin-at-dfinity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very nice, thank you.

Copy link
Copy Markdown

@derlerd-dfinity derlerd-dfinity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks a lot!

@marc0olo
Copy link
Copy Markdown
Member

marc0olo commented Mar 2, 2026

#55 just merged and changed the frontmatter format for all skills. This PR now has merge conflicts in skills/multi-canister/SKILL.md.

The new frontmatter format is:

---
name: multi-canister
description: "..."
license: Apache-2.0
compatibility: "icp-cli >= 0.1.0"
metadata:
  title: "Multi-Canister Architecture"
  category: Architecture
---

Removed fields: version, endpoints, status, dependencies, requires, tags. See CONTRIBUTING.md for the full reference.

Please rebase on main and update the frontmatter accordingly.

@marc0olo
Copy link
Copy Markdown
Member

marc0olo commented Mar 2, 2026

Heads up: we're also adding a dedicated canister-security skill in #58 (pending — will create shortly) that covers general IC security patterns (access control, TOCTOU, anonymous principal, pre_upgrade traps, cycles, inspect_message, etc.).

The security content here in multi-canister is complementary — it frames security in the multi-canister context (reentrancy in factory patterns, production readiness for specific examples). Both should coexist. After both PRs merge, we may add a brief cross-reference from multi-canister's security section to canister-security for agents that need deeper coverage.

…iew-multi-canister-skill

# Conflicts:
#	skills/multi-canister/SKILL.md
@robin-kunzler robin-kunzler merged commit 5326e2f into main Mar 3, 2026
6 checks passed
@robin-kunzler robin-kunzler deleted the robin-kunzler/sec-review-multi-canister-skill branch March 3, 2026 11:53
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.

4 participants