feat(sdk-core): add URI validation for attachment content strings#128
Conversation
🦋 Changeset detectedLatest commit: d10642f The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds URI validation for attachment content strings via a new isValidUri utility; resolveAttachmentContent now validates string content and throws ValidationError for invalid URIs. The utility is exported from the public API and covered by unit and integration tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant HypercertOps as HypercertOperationsImpl
participant Validator as isValidUri
participant Storage as Uploader
Caller->>HypercertOps: addAttachment(content)
HypercertOps->>Validator: isValidUri(stringContent)
alt valid
Validator-->>HypercertOps: true
HypercertOps->>Storage: upload(content)
Storage-->>HypercertOps: uploadResult
HypercertOps-->>Caller: success (attachment metadata)
else invalid
Validator-->>HypercertOps: false
HypercertOps-->>Caller: throw ValidationError
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.changeset/add-content-uri-validation.md:
- Around line 1-11: Update the changeset to explicitly call out that adding URI
validation is a breaking behavioral change for pre-1.0 consumers: state that
addAttachment will now throw ValidationError for previously-accepted plain text
content, that isValidUri was added and exported from the public API, and
recommend a minor version bump (not patch) for this 0.x package; include a short
migration note for users and mention compatibility caveats for existing callers
of addAttachment and consumers relying on previous lenient behavior.
- Around line 7-8: Update the changelog sentence that describes the isValidUri
utility to hyphenate “RFC 3986-compliant”; locate the phrase referencing the
utility name isValidUri and change “RFC 3986 compliant scheme” to “RFC
3986-compliant scheme” so the description reads: Add `isValidUri` utility to
validate URI strings have a proper scheme (supports http, https, at://, ipfs://,
and any RFC 3986-compliant scheme).
🧹 Nitpick comments (2)
packages/sdk-core/src/lib/url-utils.ts (1)
1-47: Align URI validation with a Zod schema for runtime validation consistency.This is runtime validation logic; consider encapsulating it in a Zod schema and using
safeParseto backisValidUri, keeping validation patterns consistent across the SDK.♻️ Suggested refactor
+import { z } from "zod"; + /** * Regular expression to match a valid URI with a scheme. * @@ const URI_SCHEME_REGEX = /^[a-zA-Z][a-zA-Z0-9+\-.]*:/; + +const UriSchema = z.string().refine((value) => { + if (!value) return false; + try { + new URL(value); + return true; + } catch { + return URI_SCHEME_REGEX.test(value); + } +}, { message: "Invalid URI with scheme" }); @@ export function isValidUri(uri: string): boolean { - if (!uri) return false; - - // First, try the native URL constructor (handles http, https, ftp, data, mailto, etc.) - try { - new URL(uri); - return true; - } catch { - // URL constructor failed — fall through to scheme regex check - } - - // For non-standard schemes (e.g., at://, ipfs://) that URL doesn't support, - // validate that the string at least has a valid URI scheme prefix. - return URI_SCHEME_REGEX.test(uri); + return UriSchema.safeParse(uri).success; }As per coding guidelines: Use Zod schemas for runtime validation.
packages/sdk-core/tests/lib/url-utils.test.ts (1)
1-69: Consider reusing shared fixtures for URI samples (if available).If
tests/utils/fixtures.tsalready includes URI samples, referencing them here would align with the shared-fixture convention and reduce duplication.As per coding guidelines: Use fixtures from tests/utils/fixtures.ts in test files.
Kzoeps
force-pushed
the
97-add-integrity-check-for-evidence-content-string-being-a-uri
branch
from
fbcc10d to
28fdb41
Compare
Kzoeps
force-pushed
the
97-add-integrity-check-for-evidence-content-string-being-a-uri
branch
from
28fdb41 to
d10642f
Compare
1 participant
Summary
isValidUriutility to validate that content URI strings have a proper scheme (http, https, at://, ipfs://, etc.)addAttachmentnow throwsValidationErrorwhen content strings are not valid URIsisValidUrifrom the public API for consumer reuseChanges
New:
isValidUriutility (src/lib/url-utils.ts)URLconstructor for standard schemes (http, https, ftp, data, mailto)at://,ipfs://)@hypercerts-org/sdk-corepublic APIUpdated:
resolveAttachmentContent(src/repository/HypercertOperationsImpl.ts)ValidationErrorwith a clear message for invalid URIsCloses #97
Summary by CodeRabbit
New Features
Behavior Change
Tests