Ship Drops Studio managed collaborative platform V4 - #7
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe PR adds Managed Platform V4 infrastructure, enterprise controls, capability-aware platform pages, runtime skill selection, managed project generation, responsive navigation, documentation, and extensive unit, integration, accessibility, and end-to-end tests. ChangesPlatform surfaces
Runtime skills
Enterprise platform
Managed platform
Generated templates
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![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
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (21)
lib/agent/runtime/intelligent-builder.ts-185-194 (1)
185-194: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winGate integration capabilities on verified availability. Filter each integration before adding its kind or proxy capability; demo, setup-required, or unconfigured records currently expose provider skills to
loadRuntimeSkills. Add coverage for these states.🤖 Prompt for 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. In `@lib/agent/runtime/intelligent-builder.ts` around lines 185 - 194, Update runtimeSkillCapabilities to include an integration’s kind and proxy capability only when that integration is verified as available; exclude demo, setup-required, and unconfigured records before adding capabilities. Reuse the existing integration availability/status indicators, preserve the current proxy mappings for eligible integrations, and add coverage for each excluded state.lib/enterprise-platform/organizations.ts-262-279 (1)
262-279: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winA revoked invitation can be resent.
resendInvitationonly rejects accepted invitations. Revoking an invitation is the documented way to cut off access, yet the revoked record can still be used as a template to mint a fresh valid token.🔒 Suggested guard
if (prior.acceptedAt) enterpriseError("INVITATION_REPLAY", "Accepted invitation cannot be resent."); + if (prior.revokedAt) enterpriseError("INVITATION_REVOKED", "Revoked invitation cannot be resent.");🤖 Prompt for 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. In `@lib/enterprise-platform/organizations.ts` around lines 262 - 279, Update resendInvitation to reject invitations that already have revokedAt set, alongside the existing acceptedAt guard, before calling inviteMember. Preserve the current resend flow for active, non-accepted invitations.lib/enterprise-platform/identity.ts-96-111 (1)
96-111: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
#pendingand#usedStatesgrow without bound.Expired pending requests are only detected lazily in
#assertFreshand never removed, and#usedStatesaccumulates forever. Every abandoned authorization flow leaks an entry for the process lifetime.🔒 Suggested pruning on `begin`
const stateHash = sha256(state); + const nowMs = this.#runtime.now().getTime(); + for (const [key, request] of this.#pending) { + if (nowMs > Date.parse(request.expiresAt)) this.#pending.delete(key); + } this.#pending.set(stateHash, {Also consider bounding
#usedStates(e.g. record the consumption time and drop entries past the 10-minute request window, since a state can no longer be replayed once its pending record is gone).🤖 Prompt for 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. In `@lib/enterprise-platform/identity.ts` around lines 96 - 111, Update the OIDC begin flow around `#pending` and `#usedStates` to prune expired entries before registering a new request. Remove pending records whose expiration is past the current runtime time, and remove used-state records older than the 10-minute request window (using their consumption timestamp or equivalent), while preserving replay protection for active states.lib/enterprise-platform/policies.ts-152-171 (1)
152-171: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winNumeric action operands are unvalidated, so
NaNfails open.
minimumvalidates policy-side numbers, but evaluation trusts the action payload.NaN > limitandNaNcost comparisons arefalse, so a non-finitedurationSeconds,count, orestimatedCostis reported as allowed; negativecountalso passes. Since these values reach the evaluator from untyped/JSON callers, enforce them here.🛡️ Proposed fix
const deny = (message: string): void => { allowed = false; reason = message; }; + const positiveInteger = (value: number): boolean => Number.isSafeInteger(value) && value >= 1; switch (action.action) { case "model.use": if (policy.allowedModelProviders && !policy.allowedModelProviders.includes(action.provider)) deny("Model provider is not allowed."); else if (action.model && policy.allowedModels && !policy.allowedModels.includes(action.model)) deny("Model is not allowed."); else if (policy.byokRequired && !action.byok) deny("BYOK is required."); else if (!policy.platformModelsAllowed && !action.byok) deny("Platform-funded models are disabled."); - else if ((action.estimatedCost ?? 0) > policy.maxAgentCostPerRun) deny("Agent cost exceeds policy."); + else if (action.estimatedCost !== undefined && (!Number.isFinite(action.estimatedCost) || action.estimatedCost < 0)) deny("Estimated agent cost is invalid."); + else if ((action.estimatedCost ?? 0) > policy.maxAgentCostPerRun) deny("Agent cost exceeds policy."); break; @@ case "sandbox.start": - if (action.durationSeconds > policy.maxSandboxDuration) deny("Sandbox duration exceeds policy."); + if (!positiveInteger(action.durationSeconds)) deny("Sandbox duration is invalid."); + else if (action.durationSeconds > policy.maxSandboxDuration) deny("Sandbox duration exceeds policy."); break; case "agents.parallel": - if (action.count > policy.maxParallelAgents) deny("Parallel agent count exceeds policy."); + if (!positiveInteger(action.count)) deny("Parallel agent count is invalid."); + else if (action.count > policy.maxParallelAgents) deny("Parallel agent count exceeds policy."); break;🤖 Prompt for 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. In `@lib/enterprise-platform/policies.ts` around lines 152 - 171, Validate numeric operands in the action evaluation switch before applying policy comparisons: require finite, non-negative durationSeconds, count, and estimatedCost values, and deny invalid payloads rather than allowing them through NaN comparisons. Update the relevant model.use, sandbox.start, and agents.parallel branches while preserving existing policy-limit checks for valid values.components/platform/organization-console.tsx-44-75 (1)
44-75: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winNo request timeout policy in
OrganizationConsole. Both network calls run without anAbortController/timeout, so a hung request pins the component in a state (loadingorcreating) that disables every control and offers no recovery — unlikePlatformCapabilityConsole, which bounds its fetch at 10s.
components/platform/organization-console.tsx#L44-L75: add a 10s abort to the/api/teamsGET, surface a distinct timeout message, and abort on unmount from the effect.components/platform/organization-console.tsx#L82-L88: add the same bounded signal to the workspace-creation POST socreatingalways clears.🤖 Prompt for 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. In `@components/platform/organization-console.tsx` around lines 44 - 75, Update refresh in OrganizationConsole to use a 10-second AbortController timeout for the /api/teams GET, display a distinct timeout message, and abort the request during effect cleanup on unmount. Also add the same bounded abort signal to the workspace-creation POST near the creation handler so its creating state always resolves; apply these changes at components/platform/organization-console.tsx ranges 44-75 and 82-88.lib/project-template-managed.ts-122-140 (1)
122-140: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftThe blanket
catchreports every failure assetup-required, which pushes healthy projects into the browser-local fallback.An undeclared collection name, an upstream 5xx, a
redirect: "error"rejection, and anAbortSignal.timeoutexpiry all collapse into503 setup-required. The generated client (Line 185) treats any non-OK response as "managed backend unavailable" and permanently switches to browser-local storage, so a transient upstream blip silently diverges user data from the managed store. Separate the configuration case from request failures.🛠️ Proposed fix: distinguish failure classes
async function forward(request: Request, params: Promise<{ collection: string }>, method: "GET" | "POST") { + let name: string; try { if (method === "POST" && !sameOrigin(request)) return Response.json({ state: "permission-denied" }, { status: 403 }); - const name = await collection(params); + name = await collection(params); + } catch { + return Response.json({ state: "unknown-collection" }, { status: 404, headers: { "cache-control": "private, no-store" } }); + } + try { const body = method === "POST" ? await request.text() : undefined; if (body && new TextEncoder().encode(body).byteLength > BODY_LIMIT_BYTES) return Response.json({ state: "quota-exceeded" }, { status: 413 }); const upstream = await managedBackendRequest("/v1/collections/" + encodeURIComponent(name), { method, headers: body ? { "content-type": "application/json" } : undefined, body, }); const payload = await upstream.text(); return new Response(payload, { status: upstream.status, headers: { "cache-control": "private, no-store", "content-type": upstream.headers.get("content-type") || "application/json" }, }); - } catch { - return Response.json({ state: "setup-required", message: "Managed backend is not configured; use the labelled browser-local fallback." }, { status: 503, headers: { "cache-control": "private, no-store" } }); + } catch (cause) { + const unconfigured = cause instanceof Error && cause.message.includes("capability is not configured"); + return Response.json( + unconfigured + ? { state: "setup-required", message: "Managed backend is not configured; use the labelled browser-local fallback." } + : { state: "upstream-unavailable" }, + { status: unconfigured ? 503 : 502, headers: { "cache-control": "private, no-store" } }, + ); } }🤖 Prompt for 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. In `@lib/project-template-managed.ts` around lines 122 - 140, Update the managed collection request handler around managedBackendRequest so only the missing or invalid backend configuration returns the 503 setup-required response. Let collection lookup errors, upstream non-OK responses, redirect rejections, and timeout failures retain their actual failure status or propagate instead of entering the setup-required fallback, ensuring the generated client does not switch to browser-local storage for request failures.lib/enterprise-platform/lifecycle.ts-258-258 (1)
258-258: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestore approval contract is weaker in code than in the docs. The single root cause is that production protection is inferred from one exact environment string, so the documented guarantees (unconditional approval, separate-environment default, target isolation, audit evidence) are not enforced.
lib/enterprise-platform/lifecycle.ts#L258-L258: replace thetoLowerCase() === "production"check with a protected-environment classification (configured name set or caller-supplied flag), and add the target-isolation check plus recorded audit evidence incompleteRestore.docs/platform/BACKUP_AND_RECOVERY.md#L3-L7: restate the approval/isolation/audit-evidence claims to match whatplanRestore/completeRestoreactually enforce, or update them once the code is tightened.🤖 Prompt for 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. In `@lib/enterprise-platform/lifecycle.ts` at line 258, Strengthen production-restore protection across lib/enterprise-platform/lifecycle.ts: in the approval check near completeRestore, replace the exact lowercase "production" comparison with the configured protected-environment classification or caller-supplied flag, enforce target isolation, and record audit evidence; update docs/platform/BACKUP_AND_RECOVERY.md lines 3-7 to accurately describe the approval, isolation, and audit guarantees enforced by planRestore and completeRestore.lib/enterprise-platform/branches.ts-206-216 (1)
206-216: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRestore discards the current canonical state with no recovery point.
mergeBranchsnapshots before overwriting (Line 160), butrestoreCheckpointreplacesproject.filesoutright — an incorrect restore permanently loses everything written since the target checkpoint.🛡️ Proposed fix
const checkpoint = this.#checkpoints.get(input.checkpointId); if (!checkpoint || checkpoint.projectId !== project.projectId) enterpriseError("NOT_FOUND", "Checkpoint was not found for this project."); + this.#checkpoint(project, input.actorUserId, "manual"); project.files = clone(checkpoint.files);🤖 Prompt for 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. In `@lib/enterprise-platform/branches.ts` around lines 206 - 216, Update restoreCheckpoint to create a recovery checkpoint of the current canonical project state before replacing project.files with the selected checkpoint, matching the pre-overwrite snapshot behavior in mergeBranch. Preserve the existing revision validation, target checkpoint lookup, metadata updates, and restored-state return behavior.lib/enterprise-platform/collaboration.ts-312-340 (1)
312-340: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftScope comment authorization by project/organization.
canaccepts no scope, while thread operations never validatethread.projectId; a granted user can access or mutate any thread in the store. Bind the store to a scope or authorize each operation against the thread’s project.🤖 Prompt for 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. In `@lib/enterprise-platform/collaboration.ts` around lines 312 - 340, Update the collaboration thread operations, including reply, resolve, reopen, and thread, to enforce project/organization scope before reading or mutating a thread. Bind the collaboration store to its configured scope and validate each thread’s projectId against it, or pass the thread project scope into authorization; ensure users granted access in one scope cannot access or modify threads from another.lib/managed-platform/backups.ts-111-122 (1)
111-122: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRestore is not atomic — a failure after
importStateleaves the target half-restored.
importState(Line 117) mutates the target environment beforeimportMetadataandimportReferencesrun. If either later step throws, the target keeps the restored rows/schema but loses auth metadata and secret references, and there is no rollback or compensating cleanup. Since the guard at Line 114 only blocks non-empty targets whenoverwriteis unset, a retried overwrite restore then operates on a partially-mutated environment.Stage the three imports (build then commit, or snapshot the target state and roll back on failure) so the target only changes once all components validate.
🤖 Prompt for 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. In `@lib/managed-platform/backups.ts` around lines 111 - 122, The restore flow in restore must be atomic across importState, importMetadata, and importReferences: stage and validate all three components before committing them, or snapshot the existing target and roll back on any failure. Ensure a failed restore leaves the target unchanged, including overwrite restores, while preserving the existing approval and validation checks.lib/managed-platform/runtime-services.ts-251-259 (1)
251-259: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick win
Intl.DateTimeFormatis constructed on every minute of the search loop.
zonedPartsbuilds a fresh formatter per call, and the loop at Line 293 can call it up to 527,040 times. Formatter construction is by far the most expensive part of this operation; a sparse-but-valid expression (e.g.0 0 29 2 *) will burn hundreds of thousands of constructions on a request thread. Build one formatter pernextCronOccurrencecall and reuse it.⚡ Proposed fix: hoist the formatter
-function zonedParts(date: Date, timezone: string): { minute: number; hour: number; day: number; month: number; weekday: number } { - let formatter: Intl.DateTimeFormat; - try { - formatter = new Intl.DateTimeFormat("en-US", { timeZone: timezone, minute: "2-digit", hour: "2-digit", hourCycle: "h23", day: "2-digit", month: "2-digit", weekday: "short" }); - } catch { throw new ManagedPlatformError("CRON_TIMEZONE_INVALID", "Cron timezone is invalid."); } - const parts = Object.fromEntries(formatter.formatToParts(date).map((part) => [part.type, part.value])); +function zonedFormatter(timezone: string): Intl.DateTimeFormat { + try { + return new Intl.DateTimeFormat("en-US", { timeZone: timezone, minute: "2-digit", hour: "2-digit", hourCycle: "h23", day: "2-digit", month: "2-digit", weekday: "short" }); + } catch { throw new ManagedPlatformError("CRON_TIMEZONE_INVALID", "Cron timezone is invalid."); } +} + +function zonedParts(date: Date, formatter: Intl.DateTimeFormat): { minute: number; hour: number; day: number; month: number; weekday: number } { + const parts = Object.fromEntries(formatter.formatToParts(date).map((part) => [part.type, part.value])); const weekdays: Record<string, number> = { Sun: 0, Mon: 1, Tue: 2, Wed: 3, Thu: 4, Fri: 5, Sat: 6 }; return { minute: Number(parts.minute), hour: Number(parts.hour), day: Number(parts.day), month: Number(parts.month), weekday: weekdays[parts.weekday] }; }const cursor = new Date(Math.floor(from.getTime() / 60_000) * 60_000 + 60_000); + const formatter = zonedFormatter(timezone); for (let iteration = 0; iteration < 527_040; iteration++, cursor.setUTCMinutes(cursor.getUTCMinutes() + 1)) { - const current = zonedParts(cursor, timezone); + const current = zonedParts(cursor, formatter);Skipping ahead by whole hours/days when the hour or day field cannot match would cut the worst case further, if you want to go beyond the cheap win.
Also applies to: 286-305
🤖 Prompt for 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. In `@lib/managed-platform/runtime-services.ts` around lines 251 - 259, Update the nextCronOccurrence flow and zonedParts helper so one Intl.DateTimeFormat instance is created per nextCronOccurrence call and reused throughout the search loop, while preserving the existing invalid-timezone ManagedPlatformError behavior. Pass the formatter into zonedParts instead of constructing it on every invocation.lib/managed-platform/runtime-services.ts-325-339 (1)
325-339: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
due()re-fires forever — nothing ever advancesnextRunAtor setslastRunAt.
schedulesis private and the class exposes no mutator, so once a schedule is due every subsequentdue()call returns it again, and any poller driving jobs off this would enqueue duplicates indefinitely.lastRunAtandoverlapPolicyare stored but never read or written.Add an acknowledge/advance step (recomputing
nextRunAtvianextCronOccurrenceand honoringoverlapPolicy) so a completed tick moves the schedule forward.🐛 Sketch of the missing mutator
due(scope: ManagedScope): ManagedSchedule[] { return clone([...this.schedules.values()].filter((schedule) => schedule.scopeKey === scope.scopeKey && schedule.enabled && schedule.nextRunAt && Date.parse(schedule.nextRunAt) <= this.options.now().getTime())); } + + markRan(scope: ManagedScope, scheduleId: string): ManagedSchedule { + const schedule = this.schedules.get(scheduleId); + if (!schedule || schedule.scopeKey !== scope.scopeKey) throw new ManagedPlatformError("CRON_NOT_FOUND", "Cron schedule does not exist in this scope."); + const ranAt = this.options.now(); + schedule.lastRunAt = ranAt.toISOString(); + schedule.nextRunAt = schedule.enabled ? nextCronOccurrence(schedule.expression, schedule.timezone, ranAt) : null; + this.options.logs.append(scope, { category: "cron", severity: "info", action: "cron.tick", actorId: "cron-runner", requestId: schedule.id, metadata: { scheduleId: schedule.id, nextRunAt: schedule.nextRunAt } }); + return clone(schedule); + }🤖 Prompt for 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. In `@lib/managed-platform/runtime-services.ts` around lines 325 - 339, Add a public acknowledgement/advance method alongside due() that accepts a schedule identifier and completion context, updates lastRunAt, and recomputes nextRunAt with nextCronOccurrence so acknowledged ticks are not returned repeatedly. Enforce the schedule’s overlapPolicy when advancing, preserve disabled schedules as unscheduled, and return a cloned updated schedule; keep due() scoped to discovering currently due schedules.lib/managed-platform/secrets.ts-86-108 (1)
86-108: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
rotateandrevokeemit no audit entries.
createappends asecret.createlog (Line 64) but the two other lifecycle mutations are silent, so rotation/revocation cannot be reconstructed from the managed log store — whichdocs/platform/MANAGED_PLATFORM_SECURITY.mdclaims is covered by audit integrity suites.🔒️ Proposed fix
secret.versions.push(replacement); secret.status = "active"; + this.options.logs.append(scope, { category: "secret", severity: "info", action: "secret.rotate", actorId: principal.actorId, requestId: `req_${randomUUID()}`, metadata: { secretId: secret.id, name: secret.name, version: nextVersion } }); return metadata(secret); } @@ secret.status = "revoked"; const current = secret.versions.at(-1); if (current) current.revokedAt = this.options.now().toISOString(); + this.options.logs.append(scope, { category: "secret", severity: "warning", action: "secret.revoke", actorId: principal.actorId, requestId: `req_${randomUUID()}`, metadata: { secretId: secret.id, name: secret.name } }); }🤖 Prompt for 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. In `@lib/managed-platform/secrets.ts` around lines 86 - 108, Update the rotate and revoke methods to append audit entries for their successful lifecycle mutations, matching the existing secret.create logging pattern and using the appropriate secret.rotate and secret.revoke event details. Ensure entries are written only after the corresponding mutation succeeds and retain the existing permission and availability checks.lib/managed-platform/security.ts-70-74 (1)
70-74: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftApproval receipts are format-checked only, so they can be self-minted.
Any string matching
/^approval_[a-z0-9_-]{8,160}$/ipasses, including a literal likeapproval_aaaaaaaa. Since this gates production and destructive schema migrations (lib/managed-platform/data.tsLine 187), the control provides no real assurance that an approval was actually granted. Consider signing receipts with the platform key (signPayload) and verifying scope, plan checksum, approver identity, and expiry here.🤖 Prompt for 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. In `@lib/managed-platform/security.ts` around lines 70 - 74, Replace the format-only validation in requireApproval with cryptographic receipt verification using the platform signing key and signPayload-compatible verifier. Require and validate the receipt’s scope, plan checksum, approver identity, and expiry before allowing production or destructive migrations, while preserving ManagedPlatformError for invalid or missing approvals.lib/managed-platform/storage.ts-39-55 (1)
39-55: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winObject keys are not unique within a scope.
putalways inserts a newobject_<uuid>entry, so writing the samekeytwice leaves two active objects sharing that key with no version or overwrite semantics.exportMetadatathen returns duplicate keys, and any consumer resolving an object by key has no deterministic winner. Either reject/replace on existing active(scopeKey, key)or add explicit versioning.🤖 Prompt for 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. In `@lib/managed-platform/storage.ts` around lines 39 - 55, The put method currently permits multiple active objects with the same key in a scope. Update put to enforce uniqueness for each active (scope.scopeKey, input.key) pair by either rejecting duplicates or replacing the existing active object, and ensure object storage and metadata export cannot expose duplicate keys; do not add versioning unless explicitly supported by the surrounding API.lib/managed-platform/data.ts-358-362 (1)
358-362: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winPrincipal-less state helpers are public on both service classes.
importState/exportStateandexportMetadataaccept only aManagedScope— noManagedPrincipal, noassertScope, norequirePermission— unlike every other method on these classes. They are presumably internal to the backup/restore path, but as public class members they are a scope-crossing read/write primitive one refactor away from being exposed.
lib/managed-platform/data.ts#L358-L362: confirm only the backup service calls these, and either mark them internal or take a principal and enforceassertScope+ abackend.backups.managecheck;importStateshould also validate the incoming schema before replacing environment state.lib/managed-platform/storage.ts#L79-L81: apply the same treatment toexportMetadata.🤖 Prompt for 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. In `@lib/managed-platform/data.ts` around lines 358 - 362, The principal-less backup helpers are publicly callable scope-crossing primitives. In lib/managed-platform/data.ts lines 358-362, update importState/exportState to be internal if only the backup service uses them, or require a ManagedPrincipal and enforce assertScope plus the backend.backups.manage permission; also validate the incoming schema before importState replaces state. In lib/managed-platform/storage.ts lines 79-81, apply the same internalization or principal-and-authorization treatment to exportMetadata.lib/managed-platform/security.ts-76-88 (1)
76-88: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse a locale-independent comparator for canonical key ordering.
localeCompareis locale/ICU-dependent, so the same object can serialize differently across runtimes orLANGsettings. SincestableJsonfeedssha256checksums that are produced and verified in separate calls (e.g. migration plan checksum inlib/managed-platform/data.tsLines 167 and 186, unique-constraint comparison, backup hashes), a locale difference can surface as spuriousMIGRATION_TAMPERED/ integrity failures. Plain code-unit comparison is deterministic everywhere.🔧 Deterministic key ordering
return Object.fromEntries(Object.entries(value as Record<string, unknown>) - .sort(([left], [right]) => left.localeCompare(right)) + .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)) .map(([key, nested]) => [key, canonicalize(nested)]));🤖 Prompt for 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. In `@lib/managed-platform/security.ts` around lines 76 - 88, Update the key sort comparator in canonicalize to use deterministic plain code-unit ordering instead of localeCompare, while preserving recursive canonicalization and stableJson behavior.lib/managed-platform/data.ts-327-334 (1)
327-334: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUpdate idempotency fingerprint/key omit the collection name.
Unlike
create(Line 243), the update fingerprint and the map key${actorId}:update:${key}contain no collection. Reusing one idempotency key across two collections with the same row id and patch replays the first collection's record instead of raisingIDEMPOTENCY_CONFLICT, returning a record from the wrong collection.🔧 Scope update idempotency by collection
- const fingerprint = sha256(stableJson({ action: "update", id, options: { expectedRevision: options.expectedRevision }, data })); + const fingerprint = sha256(stableJson({ action: "update", collectionName, id, options: { expectedRevision: options.expectedRevision }, data })); if (options.idempotencyKey) { - const previous = state.idempotency.get(`${principal.actorId}:update:${options.idempotencyKey}`); + const previous = state.idempotency.get(`${principal.actorId}:update:${collectionName}:${options.idempotencyKey}`);Apply the same key change at Line 339.
🤖 Prompt for 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. In `@lib/managed-platform/data.ts` around lines 327 - 334, Update the update idempotency logic around the fingerprint and state.idempotency lookup to include the collection name in both the hashed input and map key, matching the create flow. Apply the same collection-scoped key construction at the related line 339 usage, preserving replay for identical requests and IDEMPOTENCY_CONFLICT for differing requests.lib/managed-platform/data.ts-286-294 (1)
286-294: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winUnknown filter operators silently fall through to
lte.
comparehas no operator whitelist: any value other than the seven handled cases hits the finalreturn left <= right. Since filters arrive as data (and are only checked forin-array shape at Line 280), a typo'd or unsupported operator returns a wrong result set instead of failing. Validate operators explicitly.🔧 Reject unsupported operators
+ const OPERATORS = new Set(["eq", "ne", "in", "gt", "gte", "lt", "lte"]); + for (const filter of filters) { + if (!OPERATORS.has(filter.operator)) throw new ManagedPlatformError("QUERY_OPERATOR", `Query operator ${filter.operator} is invalid.`); + } const compare = (left: unknown, operator: string, right: unknown) => { if (operator === "eq") return stableJson(left) === stableJson(right);🤖 Prompt for 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. In `@lib/managed-platform/data.ts` around lines 286 - 294, Update the compare function to explicitly validate the operator against the supported eq, ne, in, gt, gte, lt, and lte values before evaluating it. Reject unsupported operators instead of allowing them to fall through to the lte comparison, while preserving the existing behavior for valid operators.lib/managed-platform/data.ts-99-115 (1)
99-115: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
add-fieldbypasses collection-level schema validation.The
add-fieldbranch only checks the field name and the default value, so invariants enforced byvalidateCollectionare skipped:
- the 100-field cap (Line 73) can be exceeded by repeated
add-fieldmigrations;- the enum consistency rule (Lines 76-78) is skipped, so
{ type: "enum" }with missing/duplicateenumValuesis accepted and every later write to that field failsFIELD_TYPE_INVALID.Re-validating the mutated collection keeps all branches consistent (and matches how
add-indexre-runsvalidateCollection).🔧 Re-validate after add-field
if (operation.kind === "add-field") { if (!NAME.test(operation.field) || collection.fields[operation.field]) throw new ManagedPlatformError("FIELD_EXISTS", `Field ${operation.field} already exists or is invalid.`); - validateFieldValue(operation.field, operation.definition, operation.definition.default); - collection.fields[operation.field] = clone(operation.definition); + snapshot.collections[collection.name] = validateCollection({ + ...collection, + fields: { ...collection.fields, [operation.field]: clone(operation.definition) }, + }); } else if (operation.kind === "rename-field") {🤖 Prompt for 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. In `@lib/managed-platform/data.ts` around lines 99 - 115, Update the add-field branch in the migration operation handler to re-run validateCollection after inserting the cloned field definition, preserving the existing name and default-value checks. Ensure the mutated collection is validated before the operation completes so collection-wide limits and enum invariants match the add-index branch.lib/managed-platform/auth.ts-50-71 (1)
50-71: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAdd a per-scope/email issuance rate limit before sending codes.
Each challenge has an independent 6-digit code, so unlimited challenges provide unlimited guesses and enable email bombing; the five-attempt cap only applies per challenge. Add a configurable windowed limit before
deliverOneTimeCode(the existingbackend.auth.managecheck does not replace abuse protection).🤖 Prompt for 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. In `@lib/managed-platform/auth.ts` around lines 50 - 71, Add a configurable windowed issuance limit keyed by scope.scopeKey and normalizedEmail in requestEmailCode, checking it after validation and adapter availability but before deliverOneTimeCode. Reject requests exceeding the limit and record successful issuance timestamps so separate challenges cannot bypass the per-scope/email cap; preserve the existing permission and challenge behavior otherwise.
🟡 Minor comments (16)
lib/enterprise-platform/identity.ts-156-158 (1)
156-158: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winGroup→role resolution picks the alphabetically first mapped group, not the most privileged.
claims.groupsis sorted inissueLocalTestCode(Line 141), so a user in bothadminsandviewers-readonlyresolves by lexical order rather than privilege. Make the precedence explicit (e.g. rank againstDEFAULT_ROLE_IDS) so role assignment is predictable.🤖 Prompt for 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. In `@lib/enterprise-platform/identity.ts` around lines 156 - 158, Update the role resolution in the pending-claims flow to choose the most privileged mapped group rather than the first entry in sorted claims.groups. Rank resolved roles using the existing DEFAULT_ROLE_IDS precedence, select the highest-priority role, and retain "viewer" as the fallback when no group maps to a role.lib/enterprise-platform/identity.ts-223-241 (1)
223-241: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUnverified domain claims are permanent, and
rotateChallengeduplicates the ownership check.An organization that creates a challenge and never verifies it blocks every other organization from claiming that domain forever, since the map entry is keyed by domain and never expires. Consider releasing the claim when the challenge has expired and
verifiedAtisnull.rotateChallenge(Lines 236-241) then just forwards tocreateChallenge, which already performs the identical check — the duplicated guard can go.🤖 Prompt for 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. In `@lib/enterprise-platform/identity.ts` around lines 223 - 241, Update createChallenge to remove an existing domain entry when its expiresAt is in the past and verifiedAt is null, allowing another organization to claim the expired unverified domain; retain verified claims and active challenges. Remove the redundant ownership check and related existing lookup from rotateChallenge, leaving it to delegate directly to createChallenge.lib/enterprise-platform/organizations.ts-150-167 (1)
150-167: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWorkspace creator can be locked out of the workspace it just created.
Line 165 seeds explicit membership with the org owner only. An actor holding
workspace.managethrough a custom role (i.e.roleIdis neitherownernoradmin) fails#assertWorkspaceAccess(Line 430) for the workspace it created, so it cannot then callcreateProjectthere.🐛 Suggested fix
- this.#workspaceMembers.set(workspace.id, new Set([organization.ownerUserId])); + this.#workspaceMembers.set(workspace.id, new Set([organization.ownerUserId, input.actorUserId]));🤖 Prompt for 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. In `@lib/enterprise-platform/organizations.ts` around lines 150 - 167, Update createWorkspace to include the creating actor identified by input.actorUserId in the new workspace’s explicit membership set, while preserving the organization owner membership. Ensure custom-role users who pass `#assertPermission` retain access for subsequent workspace operations such as createProject.lib/enterprise-platform/organizations.ts-224-260 (1)
224-260: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInvitations remain acceptable after the organization is archived.
acceptInvitationnever consults the organization record, so a membership is created for an archived org (whose#effectivePermissionsreturns[]), leaving a member with no permissions and no signal why.🛡️ Suggested guard
const userId = assertSafeId(input.userId, "User id"); + if (this.#organization(invitation.organizationId).archivedAt) { + enterpriseError("INVALID_INPUT", "Archived organization cannot accept invitations."); + }🤖 Prompt for 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. In `@lib/enterprise-platform/organizations.ts` around lines 224 - 260, Update acceptInvitation to retrieve the invitation’s organization record and reject the operation when that organization is archived, before mutating invitation or membership state. Use the existing organization lookup and error-handling conventions in the class, while preserving the current validation and acceptance behavior for active organizations.lib/enterprise-platform/utils.ts-63-76 (1)
63-76: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
canonicalizecollapses non-plain objects (e.g.Date) to{}.
Object.entries(new Date())is empty, so anyDate(orMap/Set) nested in a value silently canonicalizes to{}. SincestableJsonfeeds audit integrity hashing (audit.tsLine 63) and export checksums, two events with differentDatemetadata produce identical hashes.JSON.stringifywould have preservedtoJSON()output here.♻️ Suggested normalization
function canonicalize(value: unknown): unknown { if (Array.isArray(value)) return value.map(canonicalize); if (!value || typeof value !== "object") return value; + if (typeof (value as { toJSON?: unknown }).toJSON === "function") { + return canonicalize((value as { toJSON(): unknown }).toJSON()); + } return Object.fromEntries(🤖 Prompt for 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. In `@lib/enterprise-platform/utils.ts` around lines 63 - 76, Update canonicalize to preserve non-plain objects’ JSON representations instead of converting them through Object.entries into {}. Keep recursive key sorting and undefined filtering for plain objects, while allowing values such as Date, Map, and Set to retain their established JSON.stringify/toJSON behavior so stableJson produces distinct integrity hashes.lib/project-template-managed.ts-115-119 (1)
115-119: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
new URL(origin)throws for opaque origins.Requests carrying
Origin: null(sandboxed iframe, some redirect flows) makesameOriginthrow rather than returnfalse, so the caller'scatchreportssetup-requiredinstead of denying the write. Parse defensively and treat unparsable origins as cross-origin.🛡️ Proposed fix
function sameOrigin(request: Request) { if (request.headers.get("sec-fetch-site")?.toLowerCase() === "cross-site") return false; const origin = request.headers.get("origin"); - return !origin || new URL(origin).origin === new URL(request.url).origin; + if (!origin) return true; + try { + return new URL(origin).origin === new URL(request.url).origin; + } catch { + return false; + } }🤖 Prompt for 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. In `@lib/project-template-managed.ts` around lines 115 - 119, Update sameOrigin to defensively parse the request’s origin and return false when it is opaque or otherwise invalid, including Origin: null, instead of allowing new URL(origin) to throw. Preserve the existing sec-fetch-site cross-site rejection and same-origin comparison for valid origins.lib/project-template-managed.ts-214-217 (1)
214-217: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winUnguarded
localStorage.setItemmakesaddItemreject in the fallback path.Quota exhaustion or storage-restricted browsing throws here, so the item is already in React state but the returned promise rejects — the exact opposite of a graceful fallback. Wrap the write in
try/catch.🛠️ Proposed fix
const next = [{ id: crypto.randomUUID(), title: normalized }, ...items].slice(0, 100); setItems(next); - localStorage.setItem(localKey(collection), JSON.stringify(next)); + try { localStorage.setItem(localKey(collection), JSON.stringify(next)); } catch { /* session-only fallback */ } return true;🤖 Prompt for 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. In `@lib/project-template-managed.ts` around lines 214 - 217, Wrap the localStorage.setItem call in addItem’s fallback path with try/catch so quota or storage errors do not reject after setItems succeeds. Keep the in-memory update and successful return behavior unchanged, and safely ignore or handle the storage write failure.tests/rendered-html.test.mjs-14-14 (1)
14-14: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUpdate the headline assertion to account for the
<br>element.The rendered headline splits
Build crypto appsand10x faster with AIaround<br>, so the contiguous regex cannot match the built HTML.🤖 Prompt for 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. In `@tests/rendered-html.test.mjs` at line 14, Update the headline assertion in the rendered HTML test to match the text across the inserted <br> element, while still verifying both “Build crypto apps” and “10x faster with AI” appear in the rendered output.components/platform/integration-catalog.tsx-36-38 (1)
36-38: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPublish a same-document update after session marker changes.
storagenever fires in the window that performed thesessionStoragewrite, andsessionStorageis tab-scoped. The catalog can retain “Setup required” after a connection is configured until it remounts. Subscribe to a custom connection-change event (or shared external store) and emit it from the marker writer.🤖 Prompt for 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. In `@components/platform/integration-catalog.tsx` around lines 36 - 38, Update subscribeToSessionConnections to listen for a custom connection-change event in addition to storage, and ensure its cleanup removes both listeners. Locate the session marker writer and dispatch that same event immediately after successful marker changes so the catalog refreshes within the current tab without requiring a remount.lib/enterprise-platform/collaboration.ts-73-73 (1)
73-73: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winDuplicate (identical) inserts trigger a false "unreachable operations" error.
validateOperationstolerates repeatedoperationIds whose content matches, butinsertscounts duplicates whilevisitedis a set — sovisited.size !== inserts.lengthfails on a document that renders fine (e.g. a deserialized document that was concatenated rather than merged).🐛 Proposed fix
- if (visited.size !== inserts.length) enterpriseError("COLLABORATION_OPERATION_INVALID", "Collaborative document contains unreachable operations."); + const uniqueInsertIds = new Set(inserts.map((insert) => insert.operationId)); + if (visited.size !== uniqueInsertIds.size) enterpriseError("COLLABORATION_OPERATION_INVALID", "Collaborative document contains unreachable operations.");Also applies to: 103-103
🤖 Prompt for 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. In `@lib/enterprise-platform/collaboration.ts` at line 73, Update validateOperations’ unreachable-operation comparison to count unique operation IDs rather than raw entries in inserts, matching the duplicate-tolerant behavior of validateOperations and the visited set. Preserve detection of genuinely unvisited inserts while allowing repeated identical operationId values.docs/platform/BACKUP_AND_RECOVERY.md-3-7 (1)
3-7: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winDocumented restore guarantees exceed the implementation.
planRestoreinlib/enterprise-platform/lifecycle.tsonly requires approval when the target is literallyproduction, does not enforce target isolation, andcompleteRestorerecords no audit evidence. Align the wording (or the code) so the doc is not read as a hard guarantee.🤖 Prompt for 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. In `@docs/platform/BACKUP_AND_RECOVERY.md` around lines 3 - 7, Align the restore documentation with the actual behavior of planRestore and completeRestore: do not claim that approval is required for every production-destructive target, that restore targets are isolated, or that audit evidence is recorded unless those guarantees are implemented. Update the statements describing approval, target isolation, and recovery verification to accurately reflect the current lifecycle behavior.lib/enterprise-platform/credentials.ts-108-116 (1)
108-116: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
rotateTokenaccepts expired tokens.Only
revokedAtblocks rotation, so an expired token can be silently exchanged for a fresh valid one — the expiry boundary enforced inauthenticate(Line 146) is bypassed via rotation.🔐 Proposed fix
const prior = this.#token(input.tokenId); if (prior.revokedAt) enterpriseError("TOKEN_REVOKED", "API token is already revoked."); + if (this.#runtime.now().getTime() > Date.parse(prior.expiresAt)) enterpriseError("TOKEN_EXPIRED", "Expired API tokens cannot be rotated.");🤖 Prompt for 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. In `@lib/enterprise-platform/credentials.ts` around lines 108 - 116, Update rotateToken to reject tokens whose expiresAt is at or before the current runtime time, in addition to the existing revokedAt check. Use the same expiry-boundary behavior and runtime clock as authenticate, before issuing the replacement token; leave valid, non-revoked token rotation unchanged.lib/managed-platform/security.ts-122-125 (1)
122-125: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winRedact before truncating.
Truncation at 2,000 chars runs first, so a secret spanning the cut is sliced and its surviving prefix may no longer match
SECRET_VALUE, leaking partial credential material into logs. Swapping the order removes that window.🔒 Reorder redaction and truncation
function sanitizeString(value: string): string { - const bounded = value.length > 2_000 ? `${value.slice(0, 2_000)}…[TRUNCATED]` : value; - return bounded.replace(SECRET_VALUE, "[REDACTED]"); + const redacted = value.replace(SECRET_VALUE, "[REDACTED]"); + return redacted.length > 2_000 ? `${redacted.slice(0, 2_000)}…[TRUNCATED]` : redacted; }🤖 Prompt for 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. In `@lib/managed-platform/security.ts` around lines 122 - 125, Update sanitizeString so SECRET_VALUE redaction occurs before the 2,000-character truncation, ensuring secrets crossing the truncation boundary cannot leak partial credential material while preserving the existing truncation marker and output behavior.tests/managed-platform-services.test.mjs-3-12 (2)
3-12: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRaise the minimum Node.js engine to
>=22.15.0or avoidmodule.registerHooks; the current>=22.13.0range includes unsupported Node.js versions 22.13–22.14.🤖 Prompt for 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. In `@tests/managed-platform-services.test.mjs` around lines 3 - 12, Update the project’s Node.js engine requirement to >=22.15.0 wherever the current >=22.13.0 constraint is declared, so the registerHooks usage in the test setup only runs on supported versions.
3-12: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAvoid
registerHookswhile Node 22 remains supported.package.jsondeclaresnode >=22.13.0, butmodule.registerHookswas introduced in Node 23.5.0 and is unavailable on Node 22, so this test fails during import on a supported runtime.🤖 Prompt for 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. In `@tests/managed-platform-services.test.mjs` around lines 3 - 12, Replace the registerHooks-based alias resolution in the test setup with a mechanism supported by Node 22.13.0, and remove the dependency on module.registerHooks while preserving the existing "`@/`..." TypeScript path resolution behavior used by the tests.lib/managed-platform/platform.ts-67-78 (1)
67-78: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winAuthorize before backup verification
dataCore.applyrechecksassertScopeandrequirePermission, so this does not bypass authorization. However, unauthorized callers can still distinguish unknown, corrupt, wrong-scope, and valid backup IDs before those checks. Move the scope and permission checks ahead ofbackups.verifyForScope.🤖 Prompt for 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. In `@lib/managed-platform/platform.ts` around lines 67 - 78, Update the schema.apply method to perform the existing scope and permission authorization checks before calling backups.verifyForScope for destructive production plans. Preserve the current backup-required error and dataCore.apply flow, using the same authorization mechanisms already enforced by dataCore.apply.
| inviteMember(input: { | ||
| actorUserId: string; | ||
| organizationId: string; | ||
| workspaceId?: string; | ||
| email: string; | ||
| roleId: string; | ||
| expiresInMs: number; | ||
| }): { invitation: InvitationRecord; token: string } { | ||
| this.#assertPermission(input.actorUserId, input.organizationId, "members.manage"); | ||
| if (input.workspaceId && this.#workspace(input.workspaceId).organizationId !== input.organizationId) { | ||
| enterpriseError("TENANT_MISMATCH", "Invitation workspace belongs to another organization."); | ||
| } | ||
| this.#permissionsForRole(input.organizationId, input.roleId); | ||
| if (!Number.isSafeInteger(input.expiresInMs) || input.expiresInMs < 1_000 || input.expiresInMs > 30 * 86_400_000) { | ||
| enterpriseError("INVALID_INPUT", "Invitation expiry is invalid."); | ||
| } | ||
| const token = this.#runtime.token(); | ||
| if (token.length < 32) enterpriseError("INVALID_INPUT", "Invitation token entropy is insufficient."); | ||
| const now = this.#runtime.now(); | ||
| const invitation: InvitationRecord = { | ||
| id: assertSafeId(this.#runtime.id("invitation"), "Invitation id"), | ||
| organizationId: input.organizationId, | ||
| ...(input.workspaceId ? { workspaceId: input.workspaceId } : {}), | ||
| email: normalizeEmail(input.email), | ||
| roleId: input.roleId, | ||
| tokenHash: sha256(token), | ||
| expiresAt: iso(new Date(now.getTime() + input.expiresInMs)), | ||
| revokedAt: null, | ||
| acceptedAt: null, | ||
| acceptedByUserId: null, | ||
| replacedInvitationId: null, | ||
| resendCount: 0, | ||
| createdAt: iso(now), | ||
| }; | ||
| this.#invitations.set(invitation.id, invitation); | ||
| return { invitation: clone(invitation), token }; | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win
Privilege escalation: inviteMember doesn't bound the invited role by the actor's own permissions.
createCustomRole (Lines 329-331) correctly refuses to grant permissions the creator lacks, but inviteMember only requires members.manage and then accepts any roleId — including owner. An admin (no billing.manage) or a custom role holding just members.manage can invite a principal with the full permission set, then act through it. Same gap applies to resendInvitation, which reuses prior.roleId.
🔒 Suggested guard
this.#assertPermission(input.actorUserId, input.organizationId, "members.manage");
if (input.workspaceId && this.#workspace(input.workspaceId).organizationId !== input.organizationId) {
enterpriseError("TENANT_MISMATCH", "Invitation workspace belongs to another organization.");
}
- this.#permissionsForRole(input.organizationId, input.roleId);
+ const granted = this.#permissionsForRole(input.organizationId, input.roleId);
+ const actorPermissions = new Set(this.#effectivePermissions(input.actorUserId, input.organizationId));
+ if (granted.some((permission) => !actorPermissions.has(permission))) {
+ enterpriseError("PERMISSION_DENIED", "Invitation cannot grant a permission the inviter lacks.");
+ }🤖 Prompt for 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.
In `@lib/enterprise-platform/organizations.ts` around lines 186 - 222, Update
inviteMember and resendInvitation to validate the invited role’s permissions
against the acting user’s permissions before creating or resending an
invitation. Reuse the existing role-permission and actor-permission helpers,
ensuring roles such as owner cannot be granted unless the actor already has
every permission in that role; preserve the existing invitation flow after this
authorization check.
| importReferences(scope: ManagedScope, references: Array<{ id: string; name: string; allowedPurposes: SecretPurpose[]; createdAt: string }>): void { | ||
| for (const reference of references) this.secrets.set(reference.id, { ...clone(reference), scopeKey: scope.scopeKey, status: "rotation-required", versions: [] }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | 🏗️ Heavy lift
importReferences reuses source secret ids and clobbers the source scope's entries.
this.secrets is keyed by secret id only, and the imported references carry the ids from the source environment (see exportReferences at Line 111 and the restore path in lib/managed-platform/backups.ts Line 119). Restoring a backup into a different environment of the same project therefore overwrites the source environment's StoredSecret entries in place — scopeKey is reassigned to the target and versions is emptied — so resolveForRuntime starts failing for the source environment's functions/webhooks.
Mint fresh ids on import (keying by scope + id would also work) and skip/validate collisions instead of blind set.
🐛 Proposed fix: derive per-scope keys and mint new ids
importReferences(scope: ManagedScope, references: Array<{ id: string; name: string; allowedPurposes: SecretPurpose[]; createdAt: string }>): void {
- for (const reference of references) this.secrets.set(reference.id, { ...clone(reference), scopeKey: scope.scopeKey, status: "rotation-required", versions: [] });
+ for (const reference of references) {
+ const id = `secret_${randomUUID()}`;
+ this.secrets.set(id, { ...clone(reference), id, scopeKey: scope.scopeKey, status: "rotation-required", versions: [] });
+ }
}Note that consumers holding the old reference id (e.g. signingSecretId in restored webhook configuration) will need a mapping returned from this method.
🤖 Prompt for 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.
In `@lib/managed-platform/secrets.ts` around lines 114 - 116, Update
importReferences to avoid reusing source secret IDs: generate a fresh ID for
each imported reference, preserve the target scope and rotation-required state,
and prevent collisions rather than blindly overwriting existing entries in
this.secrets. Return an old-to-new ID mapping from importReferences so restore
callers such as restored webhook signingSecretId references can be rewritten to
the minted IDs.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e77075ebda
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async function forward(request: Request, params: Promise<{ collection: string }>, method: "GET" | "POST") { | ||
| try { | ||
| if (method === "POST" && !sameOrigin(request)) return Response.json({ state: "permission-denied" }, { status: 403 }); | ||
| const name = await collection(params); |
There was a problem hiding this comment.
When a generated app is deployed with DROPS_MANAGED_PROJECT_CAPABILITY, any direct client can read an allowed collection or submit a POST without an Origin header; sameOrigin() accepts the missing header and GET receives no check at all. Because the server then attaches the project bearer capability, anonymous callers can read or mutate everything that capability permits rather than operating under caller-scoped authorization. Require an authenticated project user and propagate a scoped principal before forwarding these requests.
Useful? React with 👍 / 👎.
| collection.fields[operation.to] = collection.fields[operation.from]; | ||
| delete collection.fields[operation.from]; | ||
| for (const index of collection.indexes) index.fields = index.fields.map((field) => field === operation.from ? operation.to : field); |
There was a problem hiding this comment.
When a collection already contains rows and a rename-field plan is applied, these lines rename only the schema and indexes while leaving every record in state.rows under the old key. Reads therefore continue returning the undeclared field, filters on the new field miss existing data, and required-field semantics no longer match persisted records. Move each stored value to the new key atomically as part of applying the migration.
AGENTS.md reference: AGENTS.md:L81-L83
Useful? React with 👍 / 👎.
| if (collection.indexes.some((index) => index.name === operation.index.name)) throw new ManagedPlatformError("INDEX_EXISTS", "Index already exists."); | ||
| collection.indexes.push(validateCollection({ ...collection, indexes: [...collection.indexes, operation.index] }).indexes.at(-1)!); |
There was a problem hiding this comment.
When a collection already contains duplicate values, an add-index migration with unique: true succeeds because this validates only the index definition and never checks existing rows. The environment is then committed with data that already violates its declared uniqueness constraint, even though later writes are checked. Reject the migration or require a deterministic cleanup when current rows violate the new index.
AGENTS.md reference: AGENTS.md:L81-L83
Useful? React with 👍 / 👎.
| this.#assertPermission(input.actorUserId, input.organizationId, "members.manage"); | ||
| if (input.workspaceId && this.#workspace(input.workspaceId).organizationId !== input.organizationId) { | ||
| enterpriseError("TENANT_MISMATCH", "Invitation workspace belongs to another organization."); | ||
| } | ||
| this.#permissionsForRole(input.organizationId, input.roleId); |
There was a problem hiding this comment.
An organization admin has members.manage, so this unrestricted role lookup lets the admin invite someone with roleId: "owner"; after acceptance that member receives every permission, including billing.manage, without the current owner's confirmation or transferOwnership(). Reject owner invitations and require the existing confirmed transfer path for assigning this role.
Useful? React with 👍 / 👎.
| signCapability(scope: ManagedScope, objectId: string, operation: "read" | "delete", principal: ManagedPrincipal, options: { ttlSeconds?: number } = {}): string { | ||
| assertScope(scope, principal); | ||
| requirePermission(principal, "backend.storage.manage"); | ||
| const object = this.objects.get(objectId); | ||
| if (!object || object.scopeKey !== scope.scopeKey || object.status !== "active") throw new ManagedPlatformError("OBJECT_NOT_FOUND", "Stored object does not exist."); | ||
| const ttl = options.ttlSeconds ?? 300; | ||
| if (!Number.isInteger(ttl) || ttl < 10 || ttl > 3_600) throw new ManagedPlatformError("CAPABILITY_TTL_INVALID", "Object capability lifetime is invalid."); | ||
| return signPayload({ version: 1, scopeKey: scope.scopeKey, objectId, operation, exp: Math.floor(this.options.now().getTime() / 1000) + ttl, nonce: randomUUID() }, this.options.signingKey); |
There was a problem hiding this comment.
Callers can request a signed capability whose operation is delete, but ManagedObjectStorage only implements read(), which rejects anything other than a read token, and there is no deletion consumer elsewhere in the repository. Consequently stored objects can never be removed and their count and byte quotas can never be reclaimed. Either implement verified deletion that marks the object deleted or stop issuing unusable delete capabilities.
Useful? React with 👍 / 👎.
| if ([...this.subscriptions.values()].filter((entry) => entry.scopeKey === scope.scopeKey).length >= this.options.limits.maxRealtimeSubscriptions) throw new ManagedPlatformError("REALTIME_CONNECTION_LIMIT", "Realtime connection limit exceeded."); | ||
| const id = `subscription_${randomUUID()}`; | ||
| const subscription: RealtimeSubscription = { id, scopeKey: scope.scopeKey, collection: input.collection, cursor: 0, createdAt: this.options.now().toISOString() }; | ||
| this.subscriptions.set(id, subscription); | ||
| return clone({ ...subscription, mode: this.mode }); |
There was a problem hiding this comment.
Every call to subscribe() permanently inserts an entry into subscriptions, but the service has no unsubscribe, expiry, or disconnect-cleanup path anywhere in the repository. After an environment creates maxRealtimeSubscriptions over its lifetime—even sequentially rather than concurrently—all future clients receive REALTIME_CONNECTION_LIMIT until the process restarts. Add explicit unsubscribe and stale-subscription cleanup before counting active connections.
Useful? React with 👍 / 👎.
| const managedImport = spec.presetId === "custom-product" | ||
| ? 'import { useManagedCollection } from "../lib/use-managed-collection";' | ||
| : ""; |
There was a problem hiding this comment.
Managed backend files are enabled for any preset whose prompt requests a database, auth, jobs, realtime, or related capability, but the generated component imports and uses useManagedCollection only when presetId is custom-product. A user who asks for managed persistence while starting from any of the other eleven presets receives backend metadata that the runnable product never calls. Select and integrate the managed client from requested capabilities, with category-native data behavior for each affected preset.
AGENTS.md reference: AGENTS.md:L7-L11
Useful? React with 👍 / 👎.
| key={item.href} | ||
| href={item.href} | ||
| aria-current={active === item.label ? "page" : undefined} | ||
| className={`flex min-h-11 min-w-11 shrink-0 items-center justify-center rounded-xl px-2 text-center text-xs font-semibold no-underline transition-colors focus-visible:outline-none focus-visible:ring-3 focus-visible:ring-[#316cff]/30 sm:px-4 sm:text-sm ${ |
There was a problem hiding this comment.
At the required 390 px viewport, these navigation links use Tailwind's text-xs value (12 px); they do not become text-sm until the 640 px breakpoint. Because these are visible interactive controls, the mobile platform navigation violates the repository's 14 px control-text minimum. Use text-sm at the base breakpoint.
AGENTS.md reference: AGENTS.md:L25-L25
Useful? React with 👍 / 👎.
| <div className="flex items-start justify-between gap-4"><span className="grid size-12 place-items-center rounded-2xl bg-[#eef4ff] text-[#245fe5]"><Icon className="size-6" aria-hidden="true" /></span><StatusBadge status={status}>{label}</StatusBadge></div> | ||
| <p className="mt-6 text-xs font-bold uppercase tracking-[0.12em] text-[#245fe5]">{integration.eyebrow}</p> | ||
| <h2 className="mt-2 text-xl font-semibold tracking-[-0.03em]">{integration.name}</h2> | ||
| <p className="mt-2 flex-1 text-sm leading-6 text-[#52617a]">{integration.description}</p> |
There was a problem hiding this comment.
Integration descriptions are ordinary body copy on a public catalog, but text-sm renders them at 14 px across all viewports. This falls below the repository's mandatory 16–18 px body-text range and makes the newly added catalog systematically undersized. Render these descriptions at text-base or larger.
AGENTS.md reference: AGENTS.md:L24-L24
Useful? React with 👍 / 👎.
| writeBranchFile(input: { branchId: string; path: string; content: string }): AiTaskBranch { | ||
| const branch = this.#openBranch(input.branchId); | ||
| const path = normalizeProjectPath(input.path); | ||
| if (!branch.taskScope.some((scope) => matchesScope(path, scope))) enterpriseError("BRANCH_SCOPE_DENIED", "AI branch write is outside its assigned scope."); | ||
| if (input.content.length > 1_000_000 || input.content.includes("\0")) enterpriseError("INVALID_INPUT", "Branch file content is invalid."); | ||
| branch.files[path] = input.content; | ||
| branch.updatedAt = iso(this.#runtime.now()); |
There was a problem hiding this comment.
The 5,000-file limit is enforced only when a project is initially created; repeated scoped calls to writeBranchFile() can add unlimited new paths, and a later merge installs that oversized map into the canonical project. This bypasses the branch manager's declared file-count bound and can create projects that downstream validators or persistence cannot safely handle. Reject a write that would add a new path beyond the limit and recheck the bound before merge.
AGENTS.md reference: AGENTS.md:L81-L83
Useful? React with 👍 / 👎.
1 participant
Outcome
Ships the user-visible Replit/v0-style platform surfaces and integrates the V2/V3/V4 builder, managed backend reference runtime, collaboration, enterprise controls, agent skills, evidence, and production-safe capability reporting.
Verified
Provider truth
Environment markers never become working states without runtime health evidence. Durable D1/Postgres, external realtime and enterprise OIDC remain health-gated; the generated app uses a labelled browser-local fallback when no managed capability is configured.
Release
Production promotion is authorized after preview verification. Rollback target before this PR: dpl_GQyKYxXS8dX9QdpiipK6jKxFzoP3.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation