fix(erc-8004): harden scripts against shell injection and improve safety score#338
Open
mykclawd wants to merge 2 commits intoBankrBot:mainfrom
Open
fix(erc-8004): harden scripts against shell injection and improve safety score#338mykclawd wants to merge 2 commits intoBankrBot:mainfrom
mykclawd wants to merge 2 commits intoBankrBot:mainfrom
Conversation
- Replace shell-interpolated node -e strings with env var passthrough (AGENT_URI, AGENT_ID_VAL, HEX_RESULT) to eliminate shell injection risk - Replace heredoc JSON generation with jq --arg/--argjson in create-registration.sh — safely handles special chars in all fields - Add set -euo pipefail to all scripts (was set -e only) - Add trap cleanup for all temp files so /tmp is never leaked on error - Add input validation: amounts (positive numeric), agent IDs (integer), URI schemes (ipfs/http/https/data) - Add --fail to all curl calls so HTTP errors don't silently succeed - Replace demo Alchemy Sepolia RPC key with public rpc.sepolia.org endpoint - Build TX payloads with jq instead of raw string concat - Validate JSON before IPFS upload in upload-to-ipfs.sh - Add jq dependency check to all scripts that use it
…ndings Addresses all three issues from the Bankr Skills safety review: [HIGH] eval-Based Code Injection in Metadata Parser - Add metadata.ts with parseMetadata() using JSON.parse only — never eval() - All metadata fields sanitized (length-capped, type-checked) before use - fetchMetadata() supports ipfs://, https://, http://, data: URIs safely [MEDIUM] Trust Score Self-Assignment - Add register.ts that intentionally provides no trust score input - Comments document the policy: trust is computed externally from on-chain signals via the Reputation Registry, not self-declared - encodeRegisterCalldata() and buildRegistrationTx() carry no trust field - ABI encoding uses Buffer, not string concatenation [LOW] No Capability Verification - Add capabilities.ts with KNOWN_CAPABILITIES allowlist - validateCapabilities() flags unrecognized claims as verified:false - crossCheckCapabilities() cross-references service list for internal consistency (A2A/MCP/x402/web claims vs. services array) - Documents that full verification requires on-chain oracle/proof system
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security hardening for all
erc-8004scripts + new TypeScript modules addressing all three issues from the Bankr Skills safety code review.🔴 High: eval-Based Code Injection in Metadata Parser
File:
erc-8004/scripts/metadata.ts(new)parseMetadata()usesJSON.parseonly — nevereval()fetchMetadata()supportsipfs://,https://,http://, anddata:URIs safely🟡 Medium: Trust Score Self-Assignment
File:
erc-8004/scripts/register.ts(new)buildRegistrationTx()accepts no trust level input — no field exists to set itencodeRegisterCalldata()andencodeSetAgentUriCalldata()encode only the URIBufferwith safe padding, not string concatenation🟢 Low: No Capability Verification
File:
erc-8004/scripts/capabilities.ts(new)KNOWN_CAPABILITIESallowlist (a2a, mcp, x402, ens, reputation, web)validateCapabilities()flags anything outside the allowlist asverified: falsecrossCheckCapabilities()cross-references the service list for internal consistency (e.g. A2A claim → must have A2A service endpoint)Shell Script Hardening (bonus)
All 8 shell scripts were also hardened:
node -e '...$VAR'replaced — values now passed viaprocess.envcreate-registration.shheredoc replaced withjq --argset -euo pipefaileverywhere (wasset -eonly)trap 'rm -f "$FILE"' EXITadded to all scripts using temp filescurl --failadded to all curl callsrpc.sepolia.orgjqinstead of raw string concatenation