Skip to content

fix(canister-security): correct inspect_message bypass wording; add evals#185

Open
marc0olo wants to merge 2 commits intomainfrom
fix/canister-security-inspect-message-wording
Open

fix(canister-security): correct inspect_message bypass wording; add evals#185
marc0olo wants to merge 2 commits intomainfrom
fix/canister-security-inspect-message-wording

Conversation

@marc0olo
Copy link
Copy Markdown
Member

@marc0olo marc0olo commented May 6, 2026

Summary

  • Fixes incorrect wording in pitfall Bug Report: 9 verified issues across IC Skills (build-breaking, security, broken URLs) - From Claude #1: replaces "a malicious boundary node can bypass it" with the accurate explanation that `canister_inspect_message` runs on a single replica, and it is a malicious replica node that can skip the check. Boundary nodes are not involved in this mechanism. Wording now aligns with the official IC security docs.
  • Adds `evaluations/canister-security.json` with 8 adversarial output evals and 17 trigger evals. All prompts show existing (potentially buggy) code and ask the agent to identify problems — no prompt guides toward the right answer.

Eval results

Output evals — WITH skill vs WITHOUT skill baseline
━━━ Adversarial: inspect_message as access control ━━━
  WITH skill:    5/5
  WITHOUT skill: 4/5 — omits single-replica/consensus bypass explanation

━━━ Adversarial: reentrancy guard with global boolean ━━━
  WITH skill:    5/5
  WITHOUT skill: 4/5 — recommends try/catch instead of finally for lock release

━━━ Adversarial: let _ = CallerGuard footgun ━━━
  WITH skill:    4/4
  WITHOUT skill: 4/4 (regression guard — Rust let _ semantics are general knowledge)

━━━ Adversarial: balance deducted after await ━━━
  WITH skill:    5/5
  WITHOUT skill: 4/5 — identifies TOCTOU but misses trap-case rollback (no try/finally)

━━━ Callback trap: where to release a lock ━━━
  WITH skill:    4/4
  WITHOUT skill: 1/4 — recommends try/catch (wrong), never mentions finally
                       or its IC-specific cleanup-context semantics

━━━ Adversarial: fetchRootKey in production ━━━
  WITH skill:    5/5
  WITHOUT skill: 5/5 (regression guard — widely known)

━━━ Anonymous principal check ━━━
  WITH skill:    4/4
  WITHOUT skill: 4/4 (regression guard — widely known)

━━━ Adversarial: serializing heap data in preupgrade ━━━
  WITH skill:    4/4
  WITHOUT skill: 2/4 — misses instruction-limit as failure mode,
                       never mentions persistent actor as the fix
Trigger evals
Should trigger:     9/9 correct
Should NOT trigger: 8/8 correct

… evals

Replaces incorrect "malicious boundary node" wording — inspect_message
runs on a single replica, so it is a malicious replica node (not a
boundary node) that can skip the check. Wording now matches the official
IC security docs.

Adds evaluations/canister-security.json with 8 output evals and 17
trigger evals. Strongest signal: the callback-trap/finally eval scores
4/4 with the skill vs 1/4 without — Claude defaults to try/catch (wrong)
and never surfaces the IC-specific cleanup-context semantics of finally.
@marc0olo marc0olo requested review from a team and JoshDFN as code owners May 6, 2026 16:44
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Skill Validation Report

Validating skill: /home/runner/work/icskills/icskills/skills/canister-security

Structure

  • Pass: SKILL.md found

Frontmatter

  • Pass: name: "canister-security" (valid)
  • Pass: description: (415 chars)
  • Pass: license: "Apache-2.0"
  • Pass: metadata: (2 entries)

Markdown

  • Pass: no unclosed code fences found

Tokens

File Tokens
SKILL.md body 3,773
Total 3,773

Content Analysis

Metric Value
Word count 2,244
Code block ratio 0.41
Imperative ratio 0.09
Information density 0.25
Instruction specificity 0.90
Sections 14
List items 18
Code blocks 8

Contamination Analysis

Metric Value
Contamination level low
Contamination score 0.03
Primary language category systems
Scope breadth 2
  • Warning: Language mismatch: shell (1 category differ from primary)

Result: passed

Project Checks


✓ Project checks passed for 1 skills (0 warnings)

Replace three code-writing prompts with adversarial code-review prompts
that lead agents toward wrong answers without the skill:
- Global boolean reentrancy guard (misses finally + per-caller locking)
- Balance deducted after await (TOCTOU, misses trap handling)
- Serializing heap data in preupgrade (misses instruction limit + persistent actor)

All three now show skill uplift: 5/5 vs 4/5, 5/5 vs 4/5, 4/4 vs 2/4.
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