Open
Conversation
Contributor
📝 WalkthroughWalkthroughRenames avatar/banner fields across profile-editor (DB, types, services, client), adds URL download+upload to file-manager for webhooks, introduces public file preview endpoint and isPublic file flag, updates profile-editor port/default base URL (3006→3007), adds marketplace entry and ProfessionalProfile JSON Schema, and extends dev-sandbox identities with displayName. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant FileManagerAPI as File Manager API
participant FileService
participant DB as DB/Storage
Client->>FileManagerAPI: GET /api/public/files/:id
FileManagerAPI->>FileService: getFileByIdPublic(id)
FileService->>DB: Query file by id where isPublic = true
DB-->>FileService: file record / null
alt file record found with file.url
FileManagerAPI-->>Client: 302 Redirect to file.url
else file record found with file.data
FileManagerAPI-->>Client: 200 File bytes (Content-Type, Content-Disposition, Cache-Control: public, max-age=3600)
end
alt not found or soft-deleted
FileManagerAPI-->>Client: 404 { error: "File not found" }
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 9
🧹 Nitpick comments (3)
platforms/pictique/api/src/controllers/WebhookController.ts (1)
44-48: Add logging for skipped unmapped schemas; consider validating mapping before acquiring lock.Unmapped webhooks currently return early with no log trace, making production debugging harder. While locks auto-expire after 15 seconds, moving the validation before lock acquisition keeps intent clear and avoids temporary lock contention for invalid payloads.
♻️ Suggested improvement
const mapping = Object.values(this.adapter.mapping).find( (m) => m.schemaId === schemaId ); if (!mapping) { + console.warn( + `[WebhookController] Skipping unmapped schemaId=${schemaId}, globalId=${globalId}` + ); return res.status(200).send(); } + + this.adapter.addToLockedIds(globalId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/pictique/api/src/controllers/WebhookController.ts` around lines 44 - 48, In WebhookController, move the mapping validation to occur before calling this.adapter.addToLockedIds(globalId) and add a clear log entry when mapping is missing; specifically, check the existence of mapping (the variable referenced in the snippet) at the top of the handler and if absent call the controller/logger (e.g., use this.logger or resourced logger used elsewhere in WebhookController) to record the unmapped schema and incoming metadata, then return res.status(200).send() without acquiring the lock to avoid unnecessary lock contention and make skipped webhooks observable.platforms/profile-editor/api/src/utils/file-proxy.ts (1)
36-46: Consider SSRF mitigations for URL fetching.This function downloads from arbitrary URLs provided in webhook data. While the timeout is set (15s), consider adding:
- URL scheme validation (allow only
https://)- Blocklist for internal/private IP ranges to prevent SSRF attacks
🛡️ Example validation
} else { + // Basic SSRF mitigation + const parsed = new URL(url); + if (!['http:', 'https:'].includes(parsed.protocol)) { + console.warn('Rejected non-HTTP URL:', url); + return null; + } const response = await axios.get(url, { responseType: "arraybuffer", timeout: 15_000, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/utils/file-proxy.ts` around lines 36 - 46, The download branch that calls axios.get with the variable url must validate the target before fetching to mitigate SSRF: ensure the URL has an allowed scheme (only "https"), parse the hostname and resolve it to an IP and block private/internal ranges (e.g., 10.0.0.0/8, 127.0.0.0/8, 169.254.0.0/16, 172.16.0.0/12, 192.168.0.0/16, ::1, fd00::/8, fe80::/10) and deny fetches if matched; perform these checks immediately before the axios.get call in file-proxy.ts (the block that sets responseType and timeout), and throw/return an error if validation fails. Also ensure any error path logs or surfaces the reason and does not proceed to Buffer.from(response.data) or header parsing when blocked.platforms/profile-editor/client/src/lib/utils/file-manager.ts (1)
4-4: Prefer a single source of truth for Profile Editor base URL.
API_BASEduplicates fallback logic already present inapiClient; this can drift again on future port/env changes.♻️ Suggested refactor
-const API_BASE = () => PUBLIC_PROFILE_EDITOR_BASE_URL || 'http://localhost:3007'; +const API_BASE = () => apiClient.defaults.baseURL ?? PUBLIC_PROFILE_EDITOR_BASE_URL ?? 'http://localhost:3007';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/client/src/lib/utils/file-manager.ts` at line 4, API_BASE in file-manager duplicates the base URL fallback logic; remove the local API_BASE function and consume the single source from the existing apiClient module instead (e.g., import and use the exported base-url/getBaseUrl/API_BASE symbol from apiClient). Update any usage in file-manager.ts to reference that imported symbol so the fallback logic and env precedence are maintained centrally in apiClient.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.env.example:
- Line 95: Remove the surrounding quotes from the PUBLIC_PROFILE_EDITOR_BASE_URL
entry in the .env example to satisfy dotenv-linter's QuoteCharacter rule; locate
the PUBLIC_PROFILE_EDITOR_BASE_URL="http://localhost:3007" line and change it to
an unquoted value so it reads
PUBLIC_PROFILE_EDITOR_BASE_URL=http://localhost:3007.
In `@infrastructure/dev-sandbox/src/routes/`+page.svelte:
- Around line 472-475: The current update uses selectedIndex which can be stale;
instead locate the provisioned identity by its stable key (e.g., keyId or w3id)
and update that entry immutably: find the identity in the identities array whose
key matches the provisioned key from the provisioning result, create a
shallow-copied array with that element replaced to include profile.displayName,
assign identities to the new array and call saveIdentities(newArray); update the
code around selectedIndex, identities, saveIdentities and use the provisioned
identity key (keyId/w3id) and profile.displayName to perform the backfill.
In `@platforms/file-manager/api/src/controllers/FileController.ts`:
- Around line 574-577: The publicPreview endpoint currently calls
FileService.getFileByIdPublic which exposes any non-deleted file without auth;
update publicPreview to enforce access control by either calling a service
method that verifies requester permissions (e.g.,
FileService.getFileByIdWithAccess or a new FileService.checkAccessForFile) or by
validating the returned file's visibility/owner and the requester's auth (check
file.deleted, file.visibility/permissions and req.user) and return 403/404 when
access is not allowed; reference FileController.publicPreview and
FileService.getFileByIdPublic when making the change so the controller only
serves files that pass the access check.
- Around line 570-600: In publicPreview (controller method publicPreview calling
fileService.getFileByIdPublic) add a null check for file.data before using the
legacy fallback: if file.url is falsy and file.data is null, return an
appropriate error response (e.g., 404 or 410 with a JSON error) and log the
condition; otherwise continue to set headers and send file.data. Ensure the
check references the File entity's nullable data field so you don't send null to
res.send and avoid downstream errors.
In `@platforms/file-manager/api/src/index.ts`:
- Line 83: The public GET route app.get("/api/public/files/:id",
fileController.publicPreview) delegates to FileService.getFileByIdPublic which
currently lacks access control; update the service
(FileService.getFileByIdPublic) to enforce access rules by either (A) adding an
isPublic boolean to the File entity and having getFileByIdPublic verify
file.isPublic before returning it, or (B) restricting the endpoint/service to
only allow specific kinds of files (e.g., only profile avatars/banners) by
checking file.type or file.context in getFileByIdPublic and returning 403/404
for disallowed files; ensure the controller still calls the secured service
method and return appropriate error codes when access is denied.
In `@platforms/file-manager/api/src/services/FileService.ts`:
- Around line 117-125: The getFileByIdPublic method currently returns any
non-deleted file by id; add a boolean visibility flag to the File entity (e.g.,
isPublic: boolean, default false), update the repository query in
getFileByIdPublic to include where: { id, isPublic: true } (or use
fileRepository.findOne({ where: { id, isPublic: true } })) and ensure any
existing creation/update flows (e.g., File constructor, save/update handlers)
set isPublic appropriately; also add a migration to persist the new isPublic
column and update any tests that assume public access.
In `@platforms/profile-editor/api/src/controllers/WebhookController.ts`:
- Around line 81-90: Before calling downloadUrlAndUploadToFileManager for
rawBody.data.avatarUrl or bannerUrl, validate and reject unsafe URLs to prevent
SSRF: add an isUrlAllowed(url) helper that parses the URL (new URL()), rejects
non-http/https schemes (e.g., file://), rejects hostnames like "localhost" or
literal loopback/Private/Link-local IP ranges (127.0.0.0/8, 10.0.0.0/8,
172.16.0.0/12, 192.168.0.0/16, ::1, fc00::/7, fe80::/10) and resolves DNS names
to check resolved IPs before fetching; call isUrlAllowed(...) in the
WebhookController around downloadUrlAndUploadToFileManager and skip/log and
return null for disallowed URLs so userData.avatar/banner are not set from
unsafe sources.
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts`:
- Around line 320-337: The code currently builds patch by cloning the full
existing payload (variable patch = { ...existing }) and then sends it with acl:
["*"] in UPDATE_MUTATION/CREATE_MUTATION, which unintentionally makes the entire
user envelope public; instead, modify EVaultProfileService.ts to only send the
minimal fields that need to be public (e.g., avatarUrl and bannerUrl) or create
a separate public envelope for media, and do not overwrite or set acl to ["*"]
on the entire USER_ONTOLOGY payload—preserve the existing ACL when updating the
main user record (use existing.acl if needed) or explicitly set acl only on the
new public envelope; update the branches that reference patch, UPDATE_MUTATION,
CREATE_MUTATION, userNode, existing, and eName accordingly so only intended
fields become world-readable.
---
Nitpick comments:
In `@platforms/pictique/api/src/controllers/WebhookController.ts`:
- Around line 44-48: In WebhookController, move the mapping validation to occur
before calling this.adapter.addToLockedIds(globalId) and add a clear log entry
when mapping is missing; specifically, check the existence of mapping (the
variable referenced in the snippet) at the top of the handler and if absent call
the controller/logger (e.g., use this.logger or resourced logger used elsewhere
in WebhookController) to record the unmapped schema and incoming metadata, then
return res.status(200).send() without acquiring the lock to avoid unnecessary
lock contention and make skipped webhooks observable.
In `@platforms/profile-editor/api/src/utils/file-proxy.ts`:
- Around line 36-46: The download branch that calls axios.get with the variable
url must validate the target before fetching to mitigate SSRF: ensure the URL
has an allowed scheme (only "https"), parse the hostname and resolve it to an IP
and block private/internal ranges (e.g., 10.0.0.0/8, 127.0.0.0/8,
169.254.0.0/16, 172.16.0.0/12, 192.168.0.0/16, ::1, fd00::/8, fe80::/10) and
deny fetches if matched; perform these checks immediately before the axios.get
call in file-proxy.ts (the block that sets responseType and timeout), and
throw/return an error if validation fails. Also ensure any error path logs or
surfaces the reason and does not proceed to Buffer.from(response.data) or header
parsing when blocked.
In `@platforms/profile-editor/client/src/lib/utils/file-manager.ts`:
- Line 4: API_BASE in file-manager duplicates the base URL fallback logic;
remove the local API_BASE function and consume the single source from the
existing apiClient module instead (e.g., import and use the exported
base-url/getBaseUrl/API_BASE symbol from apiClient). Update any usage in
file-manager.ts to reference that imported symbol so the fallback logic and env
precedence are maintained centrally in apiClient.
🪄 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: f57c6f50-44d9-4a9a-8d3e-2eb99e284c04
📒 Files selected for processing (26)
.env.exampleinfrastructure/dev-sandbox/src/routes/+page.svelteplatforms/file-manager/api/src/controllers/FileController.tsplatforms/file-manager/api/src/index.tsplatforms/file-manager/api/src/services/FileService.tsplatforms/marketplace/client/client/src/data/apps.jsonplatforms/marketplace/client/client/src/pages/app-detail.tsxplatforms/pictique/api/src/controllers/WebhookController.tsplatforms/profile-editor/api/src/controllers/ProfileController.tsplatforms/profile-editor/api/src/controllers/WebhookController.tsplatforms/profile-editor/api/src/database/entities/User.tsplatforms/profile-editor/api/src/database/migrations/1775600000000-RenameAvatarBannerColumns.tsplatforms/profile-editor/api/src/index.tsplatforms/profile-editor/api/src/services/EVaultProfileService.tsplatforms/profile-editor/api/src/services/EVaultSyncService.tsplatforms/profile-editor/api/src/services/UserSearchService.tsplatforms/profile-editor/api/src/types/profile.tsplatforms/profile-editor/api/src/utils/file-proxy.tsplatforms/profile-editor/api/src/web3adapter/mappings/professional-profile.mapping.jsonplatforms/profile-editor/api/src/web3adapter/mappings/user.mapping.jsonplatforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelteplatforms/profile-editor/client/src/lib/stores/discovery.tsplatforms/profile-editor/client/src/lib/stores/profile.tsplatforms/profile-editor/client/src/lib/utils/axios.tsplatforms/profile-editor/client/src/lib/utils/file-manager.tsservices/ontology/schemas/professionalProfile.json
💤 Files with no reviewable changes (2)
- platforms/profile-editor/api/src/web3adapter/mappings/user.mapping.json
- platforms/profile-editor/api/src/web3adapter/mappings/professional-profile.mapping.json
Comment on lines
+574
to
+577
| publicPreview = async (req: Request, res: Response) => { | ||
| try { | ||
| const { id } = req.params; | ||
| const file = await this.fileService.getFileByIdPublic(id); |
Contributor
There was a problem hiding this comment.
Security concern inherited from service layer.
This endpoint exposes the security issue identified in FileService.getFileByIdPublic — any non-deleted file becomes publicly accessible without authorization. See the review comment on FileService.ts for the recommended fix.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/file-manager/api/src/controllers/FileController.ts` around lines
574 - 577, The publicPreview endpoint currently calls
FileService.getFileByIdPublic which exposes any non-deleted file without auth;
update publicPreview to enforce access control by either calling a service
method that verifies requester permissions (e.g.,
FileService.getFileByIdWithAccess or a new FileService.checkAccessForFile) or by
validating the returned file's visibility/owner and the requester's auth (check
file.deleted, file.visibility/permissions and req.user) and return 403/404 when
access is not allowed; reference FileController.publicPreview and
FileService.getFileByIdPublic when making the change so the controller only
serves files that pass the access check.
| app.post("/api/auth", authController.login); | ||
| app.get("/api/auth/sessions/:id", authController.sseStream); | ||
| app.post("/api/webhook", webhookController.handleWebhook); | ||
| app.get("/api/public/files/:id", fileController.publicPreview); |
Contributor
There was a problem hiding this comment.
Public route requires access control in service layer.
This route correctly bypasses authentication middleware, but the underlying FileService.getFileByIdPublic has no access control mechanism. As noted in the FileService review, this exposes all files publicly.
Before merging, either:
- Add an
isPublicflag to the File entity and check it ingetFileByIdPublic - Restrict this endpoint to specific file types or contexts (e.g., only profile avatars/banners)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/file-manager/api/src/index.ts` at line 83, The public GET route
app.get("/api/public/files/:id", fileController.publicPreview) delegates to
FileService.getFileByIdPublic which currently lacks access control; update the
service (FileService.getFileByIdPublic) to enforce access rules by either (A)
adding an isPublic boolean to the File entity and having getFileByIdPublic
verify file.isPublic before returning it, or (B) restricting the
endpoint/service to only allow specific kinds of files (e.g., only profile
avatars/banners) by checking file.type or file.context in getFileByIdPublic and
returning 403/404 for disallowed files; ensure the controller still calls the
secured service method and return appropriate error codes when access is denied.
Comment on lines
+117
to
+125
| async getFileByIdPublic(id: string): Promise<File | null> { | ||
| const file = await this.fileRepository.findOne({ | ||
| where: { id }, | ||
| }); | ||
| if (!file || file.name === SOFT_DELETED_FILE_NAME) { | ||
| return null; | ||
| } | ||
| return file; | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for isPublic or public flag in File entity
rg -n 'isPublic|public' platforms/file-manager/api/src/database/entities/File.ts
# Check if there's any authorization in the public preview flow
ast-grep --pattern 'getFileByIdPublic($_) {
$$$
}'Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Check if File.ts exists and read its structure
find platforms/file-manager -name "File.ts" -o -name "File.entity.ts" -o -name "file.entity.ts" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 137
🏁 Script executed:
# Read the File entity to see its actual properties
cat -n platforms/file-manager/api/src/database/entities/File.ts | head -100Repository: MetaState-Prototype-Project/prototype
Length of output: 2400
🏁 Script executed:
# Read FileService.ts around the method to see full context and any authorization
cat -n platforms/file-manager/api/src/services/FileService.ts | sed -n '100,150p'Repository: MetaState-Prototype-Project/prototype
Length of output: 1857
🏁 Script executed:
# Search for public endpoints related to files
rg -n 'getFileByIdPublic|public/files' platforms/file-manager/api/src/controllers/ -A 5 -B 2Repository: MetaState-Prototype-Project/prototype
Length of output: 839
🏁 Script executed:
# Search for any isPublic, isPrivate, visibility, or permission fields in File entity
rg -n 'Column\|@' platforms/file-manager/api/src/database/entities/File.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Read FileController around the public endpoint
cat -n platforms/file-manager/api/src/controllers/FileController.ts | sed -n '570,600p'Repository: MetaState-Prototype-Project/prototype
Length of output: 1404
🏁 Script executed:
# Search for route decorators/middleware before the handler
rg -B 10 'getFileByIdPublic' platforms/file-manager/api/src/controllers/FileController.ts | head -30Repository: MetaState-Prototype-Project/prototype
Length of output: 413
🏁 Script executed:
# Check if there's a route definition with auth decorators
rg -B 5 'publicPreview' platforms/file-manager/api/src/controllers/FileController.ts | grep -E '@|router\.'Repository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Search for any middleware or auth decorators on the route
rg 'router.*publicPreview|app.*publicPreview|@' platforms/file-manager/api/src -A 2 -B 2Repository: MetaState-Prototype-Project/prototype
Length of output: 38106
🏁 Script executed:
# Check route definitions
find platforms/file-manager -name "*route*" -o -name "*router*" | head -10Repository: MetaState-Prototype-Project/prototype
Length of output: 121
Critical: Unauthenticated access to all files without distinguishing public vs. private.
The getFileByIdPublic method retrieves ANY non-deleted file by ID without verifying whether the file is intended to be publicly accessible. The File entity has no isPublic or visibility flag—all files in the system become accessible to anyone who obtains a file ID.
The controller's own comment acknowledges this relies on "unguessable capability tokens" (UUIDs). However, security through obscurity is insufficient; file IDs may leak through URLs, logs, client-side code, or error messages, exposing private user documents.
Add an isPublic flag to the File entity and check it:
Recommended fix
async getFileByIdPublic(id: string): Promise<File | null> {
const file = await this.fileRepository.findOne({
- where: { id },
+ where: { id, isPublic: true },
});
if (!file || file.name === SOFT_DELETED_FILE_NAME) {
return null;
}
return file;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/file-manager/api/src/services/FileService.ts` around lines 117 -
125, The getFileByIdPublic method currently returns any non-deleted file by id;
add a boolean visibility flag to the File entity (e.g., isPublic: boolean,
default false), update the repository query in getFileByIdPublic to include
where: { id, isPublic: true } (or use fileRepository.findOne({ where: { id,
isPublic: true } })) and ensure any existing creation/update flows (e.g., File
constructor, save/update handlers) set isPublic appropriately; also add a
migration to persist the new isPublic column and update any tests that assume
public access.
Comment on lines
+320
to
+337
| const patch: Record<string, unknown> = { ...existing }; | ||
| if (profile.avatar) { | ||
| patch.avatarUrl = getFileManagerPublicUrl(profile.avatar); | ||
| } | ||
| if (profile.banner) { | ||
| patch.bannerUrl = getFileManagerPublicUrl(profile.banner); | ||
| } | ||
|
|
||
| const returned = result.updateMetaEnvelope.metaEnvelope?.parsed as Record<string, unknown> | undefined; | ||
| console.log( | ||
| `[eVault WRITE OK] ${eName}: UPDATE response avatarFileId=${returned?.avatarFileId ?? "NONE"} bannerFileId=${returned?.bannerFileId ?? "NONE"} keys=[${returned ? Object.keys(returned).join(",") : "EMPTY"}]`, | ||
| ); | ||
| if (userNode) { | ||
| await client.request<UpdateResult>(UPDATE_MUTATION, { | ||
| id: userNode.id, | ||
| input: { ontology: USER_ONTOLOGY, payload: patch, acl: ["*"] }, | ||
| }); | ||
| } else { | ||
| const result = await client.request<CreateResult>(CREATE_MUTATION, { | ||
| input: { | ||
| ontology: PROFESSIONAL_PROFILE_ONTOLOGY, | ||
| payload, | ||
| acl, | ||
| }, | ||
| patch.ename = eName; | ||
| patch.displayName = profile.displayName ?? eName; | ||
| await client.request<CreateResult>(CREATE_MUTATION, { | ||
| input: { ontology: USER_ONTOLOGY, payload: patch, acl: ["*"] }, |
Contributor
There was a problem hiding this comment.
Avoid publishing the whole user envelope just to make media public.
patch copies the full existing USER_ONTOLOGY payload, and both mutations send it back with acl: ["*"]. That makes every field already present in the user record world-readable, not just avatarUrl / bannerUrl. If the goal is public media propagation, preserve the existing ACL or move the public URLs into a dedicated public envelope instead of forcing the entire user record public.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around
lines 320 - 337, The code currently builds patch by cloning the full existing
payload (variable patch = { ...existing }) and then sends it with acl: ["*"] in
UPDATE_MUTATION/CREATE_MUTATION, which unintentionally makes the entire user
envelope public; instead, modify EVaultProfileService.ts to only send the
minimal fields that need to be public (e.g., avatarUrl and bannerUrl) or create
a separate public envelope for media, and do not overwrite or set acl to ["*"]
on the entire USER_ONTOLOGY payload—preserve the existing ACL when updating the
main user record (use existing.acl if needed) or explicitly set acl only on the
new public envelope; update the branches that reference patch, UPDATE_MUTATION,
CREATE_MUTATION, userNode, existing, and eName accordingly so only intended
fields become world-readable.
Contributor
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
platforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelte (2)
50-64: Revoke blob URLs to prevent memory leaks.
URL.createObjectURL()allocates browser memory that persists until explicitly revoked or page unload. Repeated uploads will accumulate unreleased blob URLs.♻️ Proposed fix for handleAvatarUpload
uploadingAvatar = true; try { const result = await uploadFile(file); await updateProfile({ avatar: result.id }); + URL.revokeObjectURL(avatarPreview!); avatarPreview = null; toast.success('Avatar updated'); } catch { toast.error('Failed to upload avatar'); + URL.revokeObjectURL(avatarPreview!); avatarPreview = null; } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelte` around lines 50 - 64, The blob URL created with URL.createObjectURL(file) (assigned to avatarPreview) must be revoked to avoid leaks: before assigning a new avatarPreview revoke any previous URL (if avatarPreview) using URL.revokeObjectURL(avatarPreview), and after the upload completes or fails ensure you revoke the created blob URL when you clear avatarPreview (in both the catch and finally paths) so that neither successful nor failed uploads leave unreleased blob URLs; update the logic around avatarPreview in the upload flow (the code that calls uploadFile and updateProfile — e.g., the handleAvatarUpload / upload handler where avatarPreview, uploadingAvatar, uploadFile, updateProfile are used) to perform these revocations.
72-86: Same blob URL leak in banner upload.Apply the same
URL.revokeObjectURL()fix here.♻️ Proposed fix for handleBannerUpload
uploadingBanner = true; try { const result = await uploadFile(file); await updateProfile({ banner: result.id }); + URL.revokeObjectURL(bannerPreview!); bannerPreview = null; toast.success('Banner updated'); } catch { toast.error('Failed to upload banner'); + URL.revokeObjectURL(bannerPreview!); bannerPreview = null; } finally {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelte` around lines 72 - 86, The banner upload flow creates a blob URL via URL.createObjectURL(file) and never revokes it, leaking blobs; update the handler around the uploadFile/updateProfile logic (the code that sets bannerPreview and calls uploadFile and updateProfile) to call URL.revokeObjectURL(bannerPreview) when the blob URL is no longer needed (both on success and on error/finally) while ensuring you don't revoke if bannerPreview is null; reference the bannerPreview variable and use URL.revokeObjectURL alongside the existing uploadFile and updateProfile calls so the preview is cleaned up in all paths.platforms/profile-editor/api/src/utils/file-proxy.ts (1)
34-35: Edge case: MIME subtypes with suffixes produce odd extensions.For MIME types like
image/svg+xml, the extension becomessvg+xml. Consider stripping suffixes:-const ext = mimeType.split("/")[1] || "bin"; +const ext = (mimeType.split("/")[1] || "bin").split("+")[0];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/utils/file-proxy.ts` around lines 34 - 35, The current logic in file-proxy.ts uses mimeType.split("/")[1] to build ext and sets filename = `upload.${ext}`, which yields suffixes like "svg+xml"; update the code that computes ext to strip any MIME suffix after a "+" (e.g., take the substring before "+") and fall back to "bin" if empty, so ext becomes "svg" for "image/svg+xml" and filename remains `upload.${ext}`; ensure you reference the existing mimeType, ext, and filename variables when making this change.platforms/profile-editor/api/src/services/EVaultProfileService.ts (1)
290-298: UnusedUserclass in destructuring.The
Userclass is destructured on line 293 but never used within this block (it was needed in previous iterations forrepo.create). The patternrepo.create({ ename: eName })works without it sincerepois already typed.- const { repo, user: localUser, User } = await getLocalUser(eName); + const { repo, user: localUser } = await getLocalUser(eName);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around lines 290 - 298, The destructuring pulls a unused `User` symbol from getLocalUser; update the call to remove `User` from the destructuring (e.g., change "const { repo, user: localUser, User } = await getLocalUser(eName);" to "const { repo, user: localUser } = await getLocalUser(eName);") so the block uses only `repo`, `localUser`, and `u` for creating/saving the avatar/banner fields.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts`:
- Around line 129-134: The getLocalUser function uses a dynamic import of
AppDataSource and immediately calls AppDataSource.getRepository, which can throw
if the TypeORM data source is not initialized; update getLocalUser to check
AppDataSource.isInitialized (or equivalent initialization flag/method) after
import and either await AppDataSource.initialize() or throw/return a clear error
if not initialized, then proceed to call AppDataSource.getRepository(User) and
repo.findOneBy({ ename: eName }); ensure you reference the getLocalUser function
and AppDataSource and handle initialization before using getRepository.
In `@platforms/profile-editor/api/src/utils/file-proxy.ts`:
- Around line 20-46: The downloadUrlAndUploadToFileManager function currently
fetches arbitrary URLs; add strict URL validation before any network request:
ensure the URL uses only http or https scheme, parse the hostname and resolve it
to IP(s) (e.g., via dns.lookup or dns.promises.lookup) and reject if any
resolved IP is in private/loopback/169.254/Link-local ranges or is localhost,
and optionally reject if hostname is a bare IP literal in those ranges; also
defend against DNS rebinding by re-resolving the hostname immediately before the
axios.get and comparing IPs, and throw/reject early with a clear error instead
of calling axios for disallowed addresses. Use the function name
downloadUrlAndUploadToFileManager and the axios.get call site to locate where to
insert these checks.
---
Nitpick comments:
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts`:
- Around line 290-298: The destructuring pulls a unused `User` symbol from
getLocalUser; update the call to remove `User` from the destructuring (e.g.,
change "const { repo, user: localUser, User } = await getLocalUser(eName);" to
"const { repo, user: localUser } = await getLocalUser(eName);") so the block
uses only `repo`, `localUser`, and `u` for creating/saving the avatar/banner
fields.
In `@platforms/profile-editor/api/src/utils/file-proxy.ts`:
- Around line 34-35: The current logic in file-proxy.ts uses
mimeType.split("/")[1] to build ext and sets filename = `upload.${ext}`, which
yields suffixes like "svg+xml"; update the code that computes ext to strip any
MIME suffix after a "+" (e.g., take the substring before "+") and fall back to
"bin" if empty, so ext becomes "svg" for "image/svg+xml" and filename remains
`upload.${ext}`; ensure you reference the existing mimeType, ext, and filename
variables when making this change.
In
`@platforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelte`:
- Around line 50-64: The blob URL created with URL.createObjectURL(file)
(assigned to avatarPreview) must be revoked to avoid leaks: before assigning a
new avatarPreview revoke any previous URL (if avatarPreview) using
URL.revokeObjectURL(avatarPreview), and after the upload completes or fails
ensure you revoke the created blob URL when you clear avatarPreview (in both the
catch and finally paths) so that neither successful nor failed uploads leave
unreleased blob URLs; update the logic around avatarPreview in the upload flow
(the code that calls uploadFile and updateProfile — e.g., the handleAvatarUpload
/ upload handler where avatarPreview, uploadingAvatar, uploadFile, updateProfile
are used) to perform these revocations.
- Around line 72-86: The banner upload flow creates a blob URL via
URL.createObjectURL(file) and never revokes it, leaking blobs; update the
handler around the uploadFile/updateProfile logic (the code that sets
bannerPreview and calls uploadFile and updateProfile) to call
URL.revokeObjectURL(bannerPreview) when the blob URL is no longer needed (both
on success and on error/finally) while ensuring you don't revoke if
bannerPreview is null; reference the bannerPreview variable and use
URL.revokeObjectURL alongside the existing uploadFile and updateProfile calls so
the preview is cleaned up in all paths.
🪄 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: b9652a7c-3b96-4a2e-9ab7-5145524efd59
📒 Files selected for processing (12)
platforms/file-manager/api/src/index.tsplatforms/profile-editor/api/src/controllers/ProfileController.tsplatforms/profile-editor/api/src/controllers/WebhookController.tsplatforms/profile-editor/api/src/index.tsplatforms/profile-editor/api/src/services/EVaultProfileService.tsplatforms/profile-editor/api/src/services/EVaultSyncService.tsplatforms/profile-editor/api/src/services/UserSearchService.tsplatforms/profile-editor/api/src/utils/file-proxy.tsplatforms/profile-editor/client/src/lib/components/profile/ProfileHeader.svelteplatforms/profile-editor/client/src/lib/stores/discovery.tsplatforms/profile-editor/client/src/lib/stores/profile.tsplatforms/profile-editor/client/src/lib/utils/file-manager.ts
✅ Files skipped from review due to trivial changes (3)
- platforms/profile-editor/client/src/lib/utils/file-manager.ts
- platforms/profile-editor/client/src/lib/stores/profile.ts
- platforms/profile-editor/api/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- platforms/file-manager/api/src/index.ts
- platforms/profile-editor/client/src/lib/stores/discovery.ts
- platforms/profile-editor/api/src/services/UserSearchService.ts
- platforms/profile-editor/api/src/controllers/ProfileController.ts
Comment on lines
+129
to
+134
| async function getLocalUser(eName: string) { | ||
| const { AppDataSource } = await import("../database/data-source"); | ||
| const { User } = await import("../database/entities/User"); | ||
| const repo = AppDataSource.getRepository(User); | ||
| return { repo, user: await repo.findOneBy({ ename: eName }), User }; | ||
| } |
Contributor
There was a problem hiding this comment.
Dynamic imports lack initialization check.
getLocalUser dynamically imports AppDataSource but doesn't verify that it's initialized before calling getRepository. If invoked before TypeORM is ready (e.g., during startup race conditions), this will throw.
Consider adding a guard:
async function getLocalUser(eName: string) {
const { AppDataSource } = await import("../database/data-source");
+ if (!AppDataSource.isInitialized) {
+ throw new Error("Database not initialized");
+ }
const { User } = await import("../database/entities/User");
const repo = AppDataSource.getRepository(User);
return { repo, user: await repo.findOneBy({ ename: eName }), User };
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async function getLocalUser(eName: string) { | |
| const { AppDataSource } = await import("../database/data-source"); | |
| const { User } = await import("../database/entities/User"); | |
| const repo = AppDataSource.getRepository(User); | |
| return { repo, user: await repo.findOneBy({ ename: eName }), User }; | |
| } | |
| async function getLocalUser(eName: string) { | |
| const { AppDataSource } = await import("../database/data-source"); | |
| if (!AppDataSource.isInitialized) { | |
| throw new Error("Database not initialized"); | |
| } | |
| const { User } = await import("../database/entities/User"); | |
| const repo = AppDataSource.getRepository(User); | |
| return { repo, user: await repo.findOneBy({ ename: eName }), User }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around
lines 129 - 134, The getLocalUser function uses a dynamic import of
AppDataSource and immediately calls AppDataSource.getRepository, which can throw
if the TypeORM data source is not initialized; update getLocalUser to check
AppDataSource.isInitialized (or equivalent initialization flag/method) after
import and either await AppDataSource.initialize() or throw/return a clear error
if not initialized, then proceed to call AppDataSource.getRepository(User) and
repo.findOneBy({ ename: eName }); ensure you reference the getLocalUser function
and AppDataSource and handle initialization before using getRepository.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
platforms/profile-editor/api/src/utils/file-proxy.ts (3)
22-66: Good SSRF mitigation, but DNS rebinding window remains.The validation logic is comprehensive for blocking private IP ranges and localhost. However, there's a time-of-check-to-time-of-use (TOCTOU) gap: DNS is resolved during validation, but the actual
axios.getcall resolves DNS again. An attacker controlling their DNS server could return a public IP during validation, then rebind to a private IP before the fetch.Consider using a custom Axios
httpAgent/httpsAgentwith alookupoverride that re-validates the resolved IP immediately before connection.💡 Hardening suggestion (optional)
import { Agent as HttpAgent } from "http"; import { Agent as HttpsAgent } from "https"; function createSafeAgent(isHttps: boolean) { const AgentClass = isHttps ? HttpsAgent : HttpAgent; return new AgentClass({ lookup: (hostname, options, callback) => { dns.lookup(hostname, { all: true }, (err, addresses) => { if (err) return callback(err, "", 0); for (const addr of addresses) { if (isPrivateIP(addr.address)) { return callback(new Error("SSRF: private IP"), "", 0); } } callback(null, addresses[0].address, addresses[0].family); }); }, }); }Then pass
httpAgent/httpsAgentto the Axios request.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/utils/file-proxy.ts` around lines 22 - 66, The current isUrlAllowed resolves DNS once, leaving a TOCTOU/DNS rebinding window; create a helper (e.g., createSafeAgent(isHttps: boolean)) that returns an http/https Agent with a custom lookup which re-resolves the hostname and rejects any private/local addresses, then use that agent when making the actual outbound request (pass as httpAgent/httpsAgent to the axios request in the code path that calls axios.get/fetch in this file) so the IP is re-validated at connect time; keep the existing isUrlAllowed checks but enforce the final check via the agent's lookup to prevent rebinding.
95-105: Consider adding a response size limit to prevent memory exhaustion.The download fetches the entire response into memory without size limits. A malicious URL pointing to a multi-gigabyte file could exhaust server memory before the file-manager's upload limits kick in.
🛡️ Proposed fix
+const MAX_DOWNLOAD_BYTES = 10 * 1024 * 1024; // 10 MB } else { const response = await axios.get(url, { responseType: "arraybuffer", timeout: 15_000, maxRedirects: 3, + maxContentLength: MAX_DOWNLOAD_BYTES, }); + if (response.data.byteLength > MAX_DOWNLOAD_BYTES) { + console.warn("Downloaded file exceeds size limit:", url); + return null; + } buffer = Buffer.from(response.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/utils/file-proxy.ts` around lines 95 - 105, The code fetches the whole file into memory via axios.get(..., responseType: "arraybuffer") which can exhaust memory; change the download to use responseType: "stream" (axios.get(..., responseType: "stream")), add a MAX_DOWNLOAD_BYTES constant (e.g. 50 * 1024 * 1024 or environment-configurable), read the stream into a temporary accumulator while counting bytes and immediately abort/destroy the stream and throw if the byte counter exceeds MAX_DOWNLOAD_BYTES, then convert the collected chunks into Buffer (assigning to buffer) and keep using response.headers["content-type"] to derive mimeType/filename; ensure you handle stream errors and close the request to avoid leaks.
15-16: Imports placed mid-file.The
dnsandnetimports are added after function definitions. Consider moving them to the top of the file with other imports for consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/utils/file-proxy.ts` around lines 15 - 16, The dns and net imports (dns from "dns/promises" and net from "net") are declared mid-file; move these import statements up to join the other top-of-file imports in platforms/profile-editor/api/src/utils/file-proxy.ts so all imports are consolidated at the top of the module, keeping the existing import names (dns, net) unchanged and ensuring no duplicate or shadowed imports elsewhere in the file.platforms/profile-editor/api/src/services/EVaultProfileService.ts (1)
506-508: Silent failure may hide sync issues.Errors are logged but swallowed, so callers won't know if the User ontology sync failed. Consider adding metrics or structured logging to track sync failure rates in production.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around lines 506 - 508, The catch block in EVaultProfileService that currently does console.error("Failed to sync avatar/banner to User ontology:", e) should record a production metric and emit structured logs, then propagate failure to callers; update that catch to call your metrics/telemetry client (e.g., metrics.increment or telemetry.record with a key like "evault.user_sync_failed"), log structured context via your logger (include userId/profileId, operation name and the error object), and either rethrow the error or return a clear failure result so callers of the EVaultProfileService sync routine know the operation failed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts`:
- Around line 476-498: The ACL preservation is broken because userNode lacks the
acl field from the META_ENVELOPES_QUERY and MetaEnvelopeNode type; update the
GraphQL query (META_ENVELOPES_QUERY) to request acl for meta envelopes and
extend the MetaEnvelopeNode type to include acl so that the existingAcl read in
EVaultProfileService (userNode -> existingAcl) returns the real ACL; after that
the UPDATE_MUTATION call using existingAcl (or fallback) in the payload will
preserve the existing ACL instead of always defaulting to ["*"].
- Around line 499-505: The CREATE branch currently sets patch.ename/displayName
but must populate required User ontology fields and surface validation errors:
set patch.username (not ename), generate and set patch.id (UUID) and
patch.createdAt (ISO timestamp), and ensure patch.displayName defaults to
profile.displayName or patch.username; call
client.request<CreateResult>(CREATE_MUTATION, { input: { ontology:
USER_ONTOLOGY, payload: patch, acl: ["*"] } }) and capture the result, then
check result.createMetaEnvelope.errors (and log/throw if present) instead of
swallowing generic exceptions; update references to CREATE_MUTATION,
USER_ONTOLOGY, CreateResult, createMetaEnvelope.errors, and the local patch
variable to implement these changes.
---
Nitpick comments:
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts`:
- Around line 506-508: The catch block in EVaultProfileService that currently
does console.error("Failed to sync avatar/banner to User ontology:", e) should
record a production metric and emit structured logs, then propagate failure to
callers; update that catch to call your metrics/telemetry client (e.g.,
metrics.increment or telemetry.record with a key like
"evault.user_sync_failed"), log structured context via your logger (include
userId/profileId, operation name and the error object), and either rethrow the
error or return a clear failure result so callers of the EVaultProfileService
sync routine know the operation failed.
In `@platforms/profile-editor/api/src/utils/file-proxy.ts`:
- Around line 22-66: The current isUrlAllowed resolves DNS once, leaving a
TOCTOU/DNS rebinding window; create a helper (e.g., createSafeAgent(isHttps:
boolean)) that returns an http/https Agent with a custom lookup which
re-resolves the hostname and rejects any private/local addresses, then use that
agent when making the actual outbound request (pass as httpAgent/httpsAgent to
the axios request in the code path that calls axios.get/fetch in this file) so
the IP is re-validated at connect time; keep the existing isUrlAllowed checks
but enforce the final check via the agent's lookup to prevent rebinding.
- Around line 95-105: The code fetches the whole file into memory via
axios.get(..., responseType: "arraybuffer") which can exhaust memory; change the
download to use responseType: "stream" (axios.get(..., responseType: "stream")),
add a MAX_DOWNLOAD_BYTES constant (e.g. 50 * 1024 * 1024 or
environment-configurable), read the stream into a temporary accumulator while
counting bytes and immediately abort/destroy the stream and throw if the byte
counter exceeds MAX_DOWNLOAD_BYTES, then convert the collected chunks into
Buffer (assigning to buffer) and keep using response.headers["content-type"] to
derive mimeType/filename; ensure you handle stream errors and close the request
to avoid leaks.
- Around line 15-16: The dns and net imports (dns from "dns/promises" and net
from "net") are declared mid-file; move these import statements up to join the
other top-of-file imports in
platforms/profile-editor/api/src/utils/file-proxy.ts so all imports are
consolidated at the top of the module, keeping the existing import names (dns,
net) unchanged and ensuring no duplicate or shadowed imports elsewhere in the
file.
🪄 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: ba5bc904-03ea-4ce0-afcb-3db11be14534
📒 Files selected for processing (10)
.env.exampleinfrastructure/dev-sandbox/src/routes/+page.svelteplatforms/file-manager/api/src/controllers/FileController.tsplatforms/file-manager/api/src/database/entities/File.tsplatforms/file-manager/api/src/database/migrations/1775700000000-AddIsPublicToFiles.tsplatforms/file-manager/api/src/services/FileService.tsplatforms/pictique/api/src/controllers/WebhookController.tsplatforms/profile-editor/api/src/services/EVaultProfileService.tsplatforms/profile-editor/api/src/utils/file-proxy.tsplatforms/profile-editor/client/src/lib/utils/file-manager.ts
✅ Files skipped from review due to trivial changes (1)
- infrastructure/dev-sandbox/src/routes/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (5)
- platforms/profile-editor/client/src/lib/utils/file-manager.ts
- platforms/pictique/api/src/controllers/WebhookController.ts
- .env.example
- platforms/file-manager/api/src/services/FileService.ts
- platforms/file-manager/api/src/controllers/FileController.ts
Comment on lines
+476
to
+498
| const userNode = await this.findMetaEnvelopeByOntology(client, USER_ONTOLOGY); | ||
| const existing = (userNode?.parsed ?? {}) as Record<string, unknown>; | ||
| // Preserve the existing ACL; only default to public for new envelopes | ||
| const existingAcl = (userNode as any)?.acl; | ||
|
|
||
| // Only patch avatarUrl/bannerUrl — don't overwrite other User fields | ||
| const patch: Record<string, unknown> = { ...existing }; | ||
| if (profile.avatar) { | ||
| patch.avatarUrl = getFileManagerPublicUrl(profile.avatar); | ||
| } | ||
| if (profile.banner) { | ||
| patch.bannerUrl = getFileManagerPublicUrl(profile.banner); | ||
| } | ||
|
|
||
| if (userNode) { | ||
| await client.request<UpdateResult>(UPDATE_MUTATION, { | ||
| id: userNode.id, | ||
| input: { | ||
| ontology: USER_ONTOLOGY, | ||
| payload: patch, | ||
| acl: existingAcl ?? ["*"], | ||
| }, | ||
| }); |
Contributor
There was a problem hiding this comment.
ACL preservation doesn't work — field not queried.
Line 479 attempts to read existingAcl from userNode, but the META_ENVELOPES_QUERY (lines 21-39) and MetaEnvelopeNode type don't include the acl field. As a result, existingAcl is always undefined, and line 496 always falls back to ["*"], making the entire user envelope public on every sync.
Either add acl to the GraphQL query and types, or explicitly preserve only the fields you intend to make public.
🔧 Proposed fix to query ACL
const META_ENVELOPES_QUERY = `
query MetaEnvelopes($filter: MetaEnvelopeFilterInput, $first: Int, $after: String) {
metaEnvelopes(filter: $filter, first: $first, after: $after) {
edges {
cursor
node {
id
ontology
parsed
+ acl
}
}
...
`;
type MetaEnvelopeNode = {
id: string;
ontology: string;
parsed: Record<string, unknown>;
+ acl?: string[];
};Then update line 479:
-const existingAcl = (userNode as any)?.acl;
+const existingAcl = userNode?.acl;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around
lines 476 - 498, The ACL preservation is broken because userNode lacks the acl
field from the META_ENVELOPES_QUERY and MetaEnvelopeNode type; update the
GraphQL query (META_ENVELOPES_QUERY) to request acl for meta envelopes and
extend the MetaEnvelopeNode type to include acl so that the existingAcl read in
EVaultProfileService (userNode -> existingAcl) returns the real ACL; after that
the UPDATE_MUTATION call using existingAcl (or fallback) in the payload will
preserve the existing ACL instead of always defaulting to ["*"].
Comment on lines
+499
to
+505
| } else { | ||
| patch.ename = eName; | ||
| patch.displayName = profile.displayName ?? eName; | ||
| await client.request<CreateResult>(CREATE_MUTATION, { | ||
| input: { ontology: USER_ONTOLOGY, payload: patch, acl: ["*"] }, | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the User ontology schema requirements
fd -t f "user.json" --exec cat {}Repository: MetaState-Prototype-Project/prototype
Length of output: 4136
🏁 Script executed:
# Get the full context of the syncAvatarBannerToUserOntology method around lines 499-505
sed -n '480,520p' platforms/profile-editor/api/src/services/EVaultProfileService.ts | cat -nRepository: MetaState-Prototype-Project/prototype
Length of output: 1502
🏁 Script executed:
# Search for where id and createdAt might be set in this method
rg -A 5 -B 5 "createdAt|patch\.id" platforms/profile-editor/api/src/services/EVaultProfileService.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
# Check the error handling around CREATE_MUTATION
rg -A 10 "CREATE_MUTATION" platforms/profile-editor/api/src/services/EVaultProfileService.ts | head -40Repository: MetaState-Prototype-Project/prototype
Length of output: 936
🏁 Script executed:
# Look for where the patch object is initialized and what fields are assigned
sed -n '470,510p' platforms/profile-editor/api/src/services/EVaultProfileService.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1366
Set required User ontology fields on envelope creation.
The CREATE path only sets ename and displayName, but the User schema requires id, username, displayName, and createdAt. Additionally:
enameshould beusername(field name mismatch)idandcreatedAtare missing entirely- Schema validation errors in the mutation result are ignored; only generic exceptions are caught and logged
Unlike other CREATE calls in the codebase that check result.createMetaEnvelope.errors, this call will silently fail validation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@platforms/profile-editor/api/src/services/EVaultProfileService.ts` around
lines 499 - 505, The CREATE branch currently sets patch.ename/displayName but
must populate required User ontology fields and surface validation errors: set
patch.username (not ename), generate and set patch.id (UUID) and patch.createdAt
(ISO timestamp), and ensure patch.displayName defaults to profile.displayName or
patch.username; call client.request<CreateResult>(CREATE_MUTATION, { input: {
ontology: USER_ONTOLOGY, payload: patch, acl: ["*"] } }) and capture the result,
then check result.createMetaEnvelope.errors (and log/throw if present) instead
of swallowing generic exceptions; update references to CREATE_MUTATION,
USER_ONTOLOGY, CreateResult, createMetaEnvelope.errors, and the local patch
variable to implement these changes.
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
makes it so that professional profiles share avatar and banner with the rest of the social platforms, deprecates avatarfileid and bannerfileid from professional user ontologies/mappings
PER USER MIGRATION/NOTIFICATION PROBABLY NEEDED (if you have a better idea lemme know)
also file manager and profile editor db would need migrations
adds profile editor to marketplace
adds professional profile to ontologies service
allows for certain file manager uploads to be publicly accessible
BONUS: show "REAL NAME" in sandbox
Issue Number
Type of change
How the change has been tested
manual (PLEASE TEST ON PROD BEFORE DEMO)
BLABSY NOT TESTED DUE TO GOD KNOWS FIREBASE SETUP
Change checklist
Summary by CodeRabbit
New Features
Bug Fixes
Chores