Skip to content

feat(skills): add proofcore-contract-auditor for smart contract notarization - #1771

Open
ProofCore-Protocol wants to merge 4 commits into
anthropics:mainfrom
ProofCore-Protocol:add-proofcore-skills
Open

ProofCore-Protocol wants to merge 4 commits into
anthropics:mainfrom
ProofCore-Protocol:add-proofcore-skills

Conversation

@ProofCore-Protocol

Copy link
Copy Markdown

Summary

Adds proofcore-contract-auditor, an Agent Skill for Web3 developers that performs automated static analysis of Solidity and Rust smart contracts and anchors cryptographic audit proofs onto the public TON Blockchain using ProofCore's zero-storage Merkle protocol.

What this skill does

  • Security Audit: Scans .sol and .rs files for common vulnerabilities (reentrancy, overflow, access control).
  • Data Boundary Isolation: Explicitly isolates untrusted contract source code as passive data to prevent indirect prompt injection.
  • Cryptographic Notarization: Generates an immutable SHA-256 Merkle root of the audit report and anchors it on-chain without storing proprietary contract code on remote servers.
  • Safe Execution & Fallback: Binds natively to mcp__proofcore__seal_content if an MCP server is configured, with a safe file-based Python fallback (scripts/anchor.py) that prevents shell command injection.

Verification & Testing

  • Validated against the agentskills.io standard format.
  • Tested locally with Claude Code CLI.
  • No root or elevated privileges required.

@98zc5g5jyw-arch

Copy link
Copy Markdown

Reviewed the new skill as a fresh pass (head 1b03fa4) — thanks for the clear write-up. The mechanical checks pass, but the service integration needs resolving before this fits this repo.

Checks that pass

  • ✅ Layout & frontmatter: skills/proofcore-contract-auditor/, kebab-case name matching the directory, 241-char description — quick_validate.py → "Skill is valid!".
  • ✅ scripts/anchor.py compiles, is stdlib-only, does no shell interpolation (a single urlopen call), and sets a request timeout.
  • ✅ Scope: only the two new files; nothing else touched.
  • ✅ The "treat *.sol / *.rs as passive data" isolation note is the right instinct.

To address

  1. The notarization step is load-bearing and uploads by default. Phase 2 says to anchor "as part of the requested audit workflow", i.e. every audit is expected to send the report off-machine; anchor.py then POSTs the report content (payload.content, not just a digest) to api.proofcore.org, uploading whichever path it is given. The "zero-storage" property isn't verifiable client-side, and the Tertiary Route has users wire in ProofCore's MCP server. The rest of ./skills stays local-first — I couldn't find another skill that requires a specific external commercial service to complete its core workflow. Suggestion: make anchoring explicitly opt-in (compute the SHA-256 locally, show it, upload only after an explicit user confirmation) and state plainly in the skill what leaves the machine; or ship ProofCore as your own plugin/marketplace entry rather than inside anthropics/skills.
  2. The npx target isn't published yet. npx @proofcore/mcp-server currently 404s on the npm registry (checked registry.npmjs.org and unpkg today), so the third route can't work — and instructions in a widely-used repo pointing npx at an unpublished scoped package are a dependency-confusion hazard. Publish it, or drop that snippet, before landing.
  3. Remote content is appended to the model's output. Phase 3 instructs appending the server-returned citation_markdown directly to the final report — please treat that response as untrusted third-party data (sanitize it, or restrict to the returned URL).
  4. Missing marketplace registration. Other new-skill PRs add a .claude-plugin/marketplace.json entry (Add resume-screening skill #1763, Add md2video-audio skill #1703); without one this skill isn't installable from the marketplace.
  5. The audit half is thin. "Run standard static analysis (e.g., Slither … Cargo …)" gives no install steps, no output parsing, no severity rules — yet allowed-tools pre-approves a broad Bash(slither *) / Bash(cargo clippy *) / Bash(python3 *) set (and is experimental in the spec, so support varies by client). Spell out the concrete commands and how findings map to the [CRITICAL…LOW] buckets — or frame the skill as notarization-first if that's the real intent.
  6. Nits: 18 of the 19 skills here also ship a LICENSE.txt (this one carries frontmatter only); ${CLAUDE_SKILL_DIR} is a documented Claude Code substitution so it works there, though no other skill in this repo relies on it; and the flow doesn't say what directory audit_report.md lives in between Phase 1 and Phase 2.

Happy to re-review once adjusted. (This repo has no CI workflows, so there's nothing CI-side to wait on.) Checked via static review + quick_validate.py + py_compile; the upload script was intentionally not executed.

@ProofCore-Protocol

Copy link
Copy Markdown
Author

Thanks for the thorough review and constructive feedback! All 6 points have been addressed in the latest commit:

  1. Local-First & Opt-In Anchoring: Phase 2 is now explicitly opt-in. Furthermore, anchor.py has been refactored to compute the SHA-256 hash strictly client-side and transmits only the digest commitment envelope ({"audit_report_sha256": "..."}). Raw contract code or full reports never leave the machine.
  2. Removed Unpublished NPX Snippet: Removed the npx snippet entirely to prevent dependency-confusion hazards. Fallback relies cleanly on the bundled stdlib-only Python script.
  3. Sanitized Citation Output: Phase 3 now treats API responses as untrusted data: only the UUID is parsed, and the citation markdown is constructed locally with a strictly formatted URL.
  4. Marketplace Registration: Added the skill entry to .claude-plugin/marketplace.json.
  5. Detailed Static Analysis Steps: Expanded Phase 1 with concrete commands (slither . --json ..., cargo clippy), explicit rules, and mapping to [CRITICAL, HIGH, MEDIUM, LOW] buckets.
  6. Nits Resolved: Added LICENSE.txt (Apache-2.0), explicitly anchored ./audit_report.md in the working directory, and removed ${CLAUDE_SKILL_DIR} in favor of standard relative paths.

Ready for your re-review!

@98zc5g5jyw-arch

Copy link
Copy Markdown

Re-reviewed the updated head (3fee1f5) — thanks for the fast turnaround. Five of the six earlier points are fully resolved; one still needs a small fix before landing.

Resolved (verified against the diff)

  • ✅ Opt-in + digest-only anchoring. Phase 2 is now explicitly opt-in, and anchor.py computes SHA-256 client-side (hashlib, chunked read) and uploads only the commitment envelope (audit_report_sha256, file_name, status). The raw report / contract source never leaves the machine — matches the "What leaves the machine" paragraph.
  • ✅ Unpublished npx target removed. No npx reference remains anywhere in the skill (grep clean).
  • ✅ Sanitized citation. Phase 3 now extracts only the deal_id and builds the citation URL locally instead of appending server-returned markdown.
  • ✅ Concrete static-analysis commands (slither . --json slither_report.json, cargo clippy --all-targets --message-format=json) with the CRITICAL…LOW mapping.
  • ✅ Nits: LICENSE.txt added, ./audit_report.md pinned to the working directory, ${CLAUDE_SKILL_DIR} dropped for relative paths.
  • ✅ quick_validate.py → "Skill is valid!" on the new head; anchor.py compiles; scope unchanged (4 files).

Still to fix

  1. The marketplace entry doesn't match the schema the other entries use. Yours: name / description / version / author / entrypoint. Every existing entry in .claude-plugin/marketplace.json uses name / description / source / strict / skills — e.g. discernment-nudge:
    { "name": "discernment-nudge", "description": "...", "source": "./", "strict": false, "skills": ["./skills/discernment-nudge"] }
    entrypoint appears nowhere else in the file, and without a skills array the marketplace loader has no skill to register, so as written the plugin likely won't resolve. Recent new-skill PRs (Add resume-screening skill #1763, Add pyxel skill for retro game development #525) follow that shape — suggest mirroring it:
    { "name": "proofcore-contract-auditor", "description": "...", "source": "./", "strict": false, "skills": ["./skills/proofcore-contract-auditor"] }
  2. Nit: LICENSE.txt is a 17-line Apache-2.0 header; every other skill here ships the full license text (~201 lines) — worth matching.
  3. Nit: the Phase 2 MCP example has nested unescaped quotes ("content": "{"audit_report_sha256": ..."}), which isn't parseable JSON — escape the inner quotes so a client can't misread the envelope.
  4. Nit: the SKILL.md says the metadata that leaves the machine is "(timestamp, report title)", but the script actually sends file_name and no timestamp — align the two.

Everything else reads well; with the marketplace entry fixed I don't see anything blocking.

@ProofCore-Protocol

Copy link
Copy Markdown
Author

Thanks again for the precise review! All remaining points have been resolved:

  1. Marketplace Schema Matched: Updated the entry in .claude-plugin/marketplace.json to strictly follow the standard shape (name, description, source: "./", strict: false, skills: ["./skills/proofcore-contract-auditor"]), matching #1763 and #525.
  2. Full Apache-2.0 License: Replaced the header snippet with the complete 201-line Apache-2.0 license text in LICENSE.txt.
  3. Escaped MCP JSON Example: Corrected the Phase 2 MCP code block to properly escape nested quotes.
  4. Metadata Alignment: Aligned the "What leaves the machine" clause in SKILL.md to precisely list (file_name, status, report title) matching anchor.py.

Ready for final landing!

@98zc5g5jyw-arch

Copy link
Copy Markdown

Verified 0254f29 against the four items from my last pass — three fully resolved, one still needs its literal escape:

Resolved ✅

  • ✅ Marketplace entry now uses the standard shape (name / description / source / strict / skills); the key set is now identical across all six entries and entrypoint is gone — JSON parses cleanly.
  • ✅ LICENSE.txt — full Apache-2.0 text now included (202 lines, APPENDIX included), consistent with the other skills in this repo.
  • ✅ Metadata clause — "What leaves the machine" now lists (file_name, status, report title), which matches anchor.py (envelope audit_report_sha256 / file_name / status, plus the title field). The old (timestamp, report title) mismatch is gone.

Still open ❌ — the escaping didn't make it into this push

  • SKILL.md L43–48 (Phase 2 MCP example) is now in a ```json fence, but the inner quotes are still raw, so the block still isn't parseable JSON (jq: `parse error: Invalid numeric literal at line 3, column 39`).

    Current:

    "content": "{"audit_report_sha256": "<hash>", "file_name": "audit_report.md", "status": "completed"}"
    

    Should be:

    "content": "{\"audit_report_sha256\": \"<hash>\", \"file_name\": \"audit_report.md\", \"status\": \"completed\"}"
    

    (escaped form is exactly what anchor.py serializes via json.dumps, so the doc will match the implementation).

quick_validate.py → "Skill is valid!" on this head. That one-line escape is the only thing left from my side.

@ProofCore-Protocol

Copy link
Copy Markdown
Author

Escaped the nested JSON string literal cleanly via json.dumps serialization.

Verified the fence parses with jq without errors now. All set!

@98zc5g5jyw-arch

Copy link
Copy Markdown

All set on my side too — re-verified the new head 9e81a99:

  • ✅ The Phase 2 snippet now parses: the fenced block round-trips cleanly through jq and json.loads, including the nested content string (previously: parse error: Invalid numeric literal at line 3, column 39).
  • ✅ Diff is scoped to that single line (1 file, +1/−1) — nothing else moved, and the escaped form matches how anchor.py serializes via json.dumps.
  • ✅ quick_validate.py → "Skill is valid!" on the updated tree — no regression.
  • ✅ Head 9e81a99ea28e57ff38bca517c60b960b24b3f0f9 matches the pushed ref.

All four items from the earlier review are now closed. LGTM — thanks for the quick turnaround!

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