Feat/w3ds file uri#965
Merged
Merged
Conversation
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ 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. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (30)
📝 WalkthroughWalkthroughThis pull request implements a complete file URI referencing and dereferencing system via the ChangesFile URI Scheme and Resolution
Sequence Diagram(s)sequenceDiagram
participant Client
participant EVaultGraphQL
participant StorageService
participant DBService
participant HTTPEndpoint
Note over Client,DBService: File Upload Flow
Client->>EVaultGraphQL: uploadFile(filename, content, contentType)
EVaultGraphQL->>StorageService: uploadObject(buffer, contentType, key)
StorageService-->>EVaultGraphQL: publicUrl
EVaultGraphQL->>DBService: createMetaEnvelope(FILE_SCHEMA_ID, {size, blobKey, publicUrl})
DBService-->>EVaultGraphQL: metaEnvelopeId
EVaultGraphQL-->>Client: {uri: "w3ds://file?id=...", metaEnvelopeId, publicUrl}
Note over Client,HTTPEndpoint: File Dereference Flow
Client->>HTTPEndpoint: GET /.files/:metaEnvelopeId (X-ENAME)
HTTPEndpoint->>DBService: getMetaEnvelope(ename, metaEnvelopeId)
DBService-->>HTTPEndpoint: {ontology: "w3ds-file-v1", data: {publicUrl}}
HTTPEndpoint-->>Client: 302 Redirect to publicUrl
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure/evault-core/src/core/http/server.ts`:
- Around line 434-443: The handler currently reads publicUrl from
metaEnvelope.parsed and calls reply.redirect(publicUrl) without validating the
scheme; update the code around publicUrl (the variable extracted from
metaEnvelope.parsed) to parse and validate the URL’s protocol (e.g., using the
URL constructor) and only allow "http:" or "https:" before calling
reply.redirect(publicUrl); if the URL is invalid or uses a disallowed scheme,
return a safe error response (404/400) instead of redirecting to prevent unsafe
non-HTTP(S) redirects.
In `@infrastructure/evault-core/src/core/protocol/graphql-server.ts`:
- Around line 1229-1275: The upload flow currently uploads via new
StorageService().uploadObject then calls this.db.storeMetaEnvelope; if
storeMetaEnvelope fails the uploaded blob is orphaned—fix by attempting
compensating delete of the uploaded object inside the catch: after catching the
error (in the catch block that currently logs "uploadFile failed:"), if the
upload key (the `key` variable used for upload) and the StorageService instance
(`storage`) exist, await a cleanup call (e.g. storage.deleteObject or
equivalent) to remove the uploaded blob, log any cleanup error separately, and
then return the original UPLOAD_FAILED error response; update code references
around StorageService, uploadObject, `key`, and storeMetaEnvelope/buildFileUri
accordingly so the cleanup runs only for upload-success/database-failure cases.
- Around line 1189-1207: The current flow decodes input.content into base64 then
checks buffer.length, which misses malformed base64; instead validate
input.content (the derived base64 variable) before calling Buffer.from: ensure
it's non-empty, length % 4 === 0, and matches a strict pattern that allows only
A-Za-z0-9+/ and at most two '=' padding characters only at the end (e.g.
/^[A-Za-z0-9+/]+={0,2}$/ with additional logic to reject '=' in the middle), and
if validation fails return the existing INVALID_CONTENT error object; replace
the buffer.length === 0 check and only call Buffer.from(base64, "base64") after
the pre-validation succeeds (use the same field/message/code payload).
In `@infrastructure/evault-core/src/core/utils/w3ds-uri.ts`:
- Around line 13-16: The buildFileUri function accepts empty/raw inputs and
interpolates them directly into the id, producing invalid URIs; update
buildFileUri(eName, metaEnvelopeId) to validate that eName and metaEnvelopeId
are non-empty (throw or return an error), normalize eName to start with "@" (but
not include slashes), URL-encode both components with encodeURIComponent, and
then construct the final string as
`w3ds://file?id=${encodedEname}/${encodedMetaEnvelopeId}` so the produced URI is
safe to dereference.
In `@infrastructure/web3-adapter/MAPPING_RULES.md`:
- Around line 97-100: The example JSON in MAPPING_RULES.md repeats the same key
"avatar" twice in one object which is ambiguous; split them into two separate
JSON examples instead of duplicate keys. Update the snippet containing
`"avatar": "__file(avatar)"` and `"avatar": "__file(avatar),avatarUri"` so each
appears in its own fenced JSON block (separate examples) and ensure the fences
are labeled ```json and closed properly; keep the exact mapping strings
("__file(avatar)" and "__file(avatar),avatarUri") so readers can copy each
example unambiguously.
In `@infrastructure/web3-adapter/src/mapper/mapper.ts`:
- Around line 293-308: The code currently falls back to raw values when handling
a __file() match in the mapper (inside toGlobal), which can write non-w3ds://
payloads; change the else branch in the fileMatch handler so it fails fast
instead of passing rawVal through: when evaultClient or ownerEvault are missing,
throw a clear Error (or return a failure) that includes the localPath and alias
(use the existing variables fileTargetKey, localPath, alias) and a message that
__file() cannot be dereferenced due to missing evault client/owner; keep the
successful path using referenceFileValue(evaultClient, ownerEvault, rawVal)
unchanged so only fully-resolved w3ds:// file references are written to result.
In `@infrastructure/web3-adapter/src/w3ds/uri.ts`:
- Line 45: The return string builds the query id without encoding and can break
on reserved characters; change the id value to a properly encoded query
component by replacing ?id=${normalisedEname}/${metaEnvelopeId} with
?id=${encodeURIComponent(`${normalisedEname}/${metaEnvelopeId}`)} (or encode
each segment separately with encodeURIComponent(normalisedEname) + '/' +
encodeURIComponent(metaEnvelopeId) if you need to preserve the slash), updating
the return in the function that uses W3DS_SCHEME and W3DS_FILE_HOST so the id
query param is always URL-safe.
In `@infrastructure/web3-adapter/W3DS_URI.md`:
- Around line 8-10: The markdown fences are missing language identifiers causing
MD040; update the three fenced blocks that contain the strings
"w3ds://file?id=@<user-ename>/<meta-envelope-id>",
"w3ds://file?id=`@alice/envelope-abc123`", and "GET /.files/:metaEnvelopeId
(header: X-ENAME: @<user-ename>)" to include appropriate languages (e.g.,
```text for the URI examples and ```http for the HTTP example) so the blocks
read like ```text ... ``` and ```http ... ``` to satisfy the lint rule.
- Around line 55-57: The documented dereference route in W3DS_URI.md currently
shows GET /files/:metaEnvelopeId (header: X-ENAME) but the implemented contract
in this PR uses GET /.files/:metaEnvelopeId; update the documentation to match
the implementation by replacing the path string `GET /files/:metaEnvelopeId`
with `GET /.files/:metaEnvelopeId` (and keep the header `X-ENAME:
@<user-ename>`), or alternatively if the intended canonical endpoint is
`/files/:metaEnvelopeId` adjust the implementation to expose that route; ensure
the document and the running contract both reference the exact same path (`GET
/.files/:metaEnvelopeId` vs `GET /files/:metaEnvelopeId`) to avoid client
mismatch.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 3d979ba8-ae9d-4a5a-b5cc-90f8e13a9adf
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
.env.exampleinfrastructure/evault-core/package.jsoninfrastructure/evault-core/src/core/http/server.tsinfrastructure/evault-core/src/core/protocol/graphql-server.tsinfrastructure/evault-core/src/core/protocol/typedefs.tsinfrastructure/evault-core/src/core/utils/w3ds-uri.tsinfrastructure/evault-core/src/services/StorageService.tsinfrastructure/web3-adapter/MAPPING_RULES.mdinfrastructure/web3-adapter/W3DS_URI.mdinfrastructure/web3-adapter/src/evault/evault.tsinfrastructure/web3-adapter/src/index.tsinfrastructure/web3-adapter/src/mapper/mapper.tsinfrastructure/web3-adapter/src/mapper/mapper.types.tsinfrastructure/web3-adapter/src/w3ds/resolver.tsinfrastructure/web3-adapter/src/w3ds/uri.test.tsinfrastructure/web3-adapter/src/w3ds/uri.ts
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.
Description of change
adds file URI scheme and fixes mappings
Issue Number
closes #964
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
__file()directiveDocumentation
Tests
Chores