fix(caip): add input validation to createCaip10AccountId#67
fix(caip): add input validation to createCaip10AccountId#67ak68a wants to merge 1 commit intoagentcommercekit:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
WalkthroughInput validation has been added to the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/caip/src/caips/caip-10.ts`:
- Around line 41-46: Add explicit string type guards before using
caip2ChainIdRegex.test(chainId) and caip10AccountAddressRegex.test(address):
check typeof chainId === "string" and typeof address === "string" and throw the
existing Error messages if those checks fail, then perform the regex tests as
currently written; update the validation logic around the caip2ChainIdRegex and
caip10AccountAddressRegex usages so non-string inputs are rejected early and
clearly (refer to the variables chainId, address and the regex constants
caip2ChainIdRegex, caip10AccountAddressRegex).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efc6a196-ca87-46a2-a4ff-64f44f7a18df
📒 Files selected for processing (2)
packages/caip/src/caips/caip-10.test.tspackages/caip/src/caips/caip-10.ts
| if (!caip2ChainIdRegex.test(chainId)) { | ||
| throw new Error(`Invalid CAIP-2 chain ID: ${chainId}`) | ||
| } | ||
| if (!caip10AccountAddressRegex.test(address)) { | ||
| throw new Error(`Invalid CAIP-10 account address: ${address}`) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify current guards around createCaip10AccountId
rg -nP --type=ts -C3 '\bcreateCaip10AccountId\s*\('
rg -nP --type=ts -C2 'caip2ChainIdRegex\.test\(chainId\)|caip10AccountAddressRegex\.test\(address\)|typeof\s+(chainId|address)\s*!==\s*"string"'
# Verify whether tests already cover non-string runtime inputs
rg -nP --type=ts -C2 'createCaip10AccountId\((null|undefined|\d+|{}|\[\])|throws for invalid' --iglob '**/*.test.ts'Repository: agentcommercekit/ack
Length of output: 7654
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get the full caip-10.ts file to see regex definitions and the function implementation
head -70 packages/caip/src/caips/caip-10.ts
echo "---"
# Get the full test file to see all test cases
cat packages/caip/src/caips/caip-10.test.tsRepository: agentcommercekit/ack
Length of output: 2983
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Check the caip-2.ts to see the full pattern
cat packages/caip/src/caips/caip-2.ts | head -40Repository: agentcommercekit/ack
Length of output: 1400
Add explicit type guards before regex checks for defensive programming.
RegExp.test(...) coerces non-strings to strings. While the current regex patterns are effective at catching typical coercions (null → "null", undefined → "undefined", 123 → "123") because they require specific separators (colon for chain ID), explicit type checks would improve code clarity and provide additional protection against edge cases.
💡 Proposed fix
export function createCaip10AccountId(
chainId: Caip2ChainId,
address: string,
): Caip10AccountId {
- if (!caip2ChainIdRegex.test(chainId)) {
- throw new Error(`Invalid CAIP-2 chain ID: ${chainId}`)
+ if (typeof chainId !== "string" || !caip2ChainIdRegex.test(chainId)) {
+ throw new Error(`Invalid CAIP-2 chain ID: ${String(chainId)}`)
}
- if (!caip10AccountAddressRegex.test(address)) {
- throw new Error(`Invalid CAIP-10 account address: ${address}`)
+ if (
+ typeof address !== "string" ||
+ !caip10AccountAddressRegex.test(address)
+ ) {
+ throw new Error(`Invalid CAIP-10 account address: ${String(address)}`)
}
return `${chainId}:${address}`
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/caip/src/caips/caip-10.ts` around lines 41 - 46, Add explicit string
type guards before using caip2ChainIdRegex.test(chainId) and
caip10AccountAddressRegex.test(address): check typeof chainId === "string" and
typeof address === "string" and throw the existing Error messages if those
checks fail, then perform the regex tests as currently written; update the
validation logic around the caip2ChainIdRegex and caip10AccountAddressRegex
usages so non-string inputs are rejected early and clearly (refer to the
variables chainId, address and the regex constants caip2ChainIdRegex,
caip10AccountAddressRegex).
Summary
chainIdagainstcaip2ChainIdRegexandaddressagainstcaip10AccountAddressRegexbefore creating the account IDcreateCaip10AccountIdpreviously performed no validation, allowing malformed chain IDs and addresses to propagate silently. The regex patterns were already defined in the module but not being used. This brings the function in line withcaip10Parts(), which already validates its input.Test plan
AI Disclosure: This PR was developed with assistance from Claude Code (Claude Opus).
Summary by CodeRabbit