Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThe PR introduces GraphQL-based bulk envelope creation and refactors the data migration flow, replacing the direct Changes
Sequence DiagramsequenceDiagram
actor Client
participant GraphQL as GraphQLServer
participant DB as DbService
participant Webhooks as Webhook Delivery
participant Logging as Operation Logging
participant Registry as Registry/JWKS
Client->>GraphQL: bulkCreateMetaEnvelopes(inputs, skipWebhooks)
GraphQL->>GraphQL: Validate X-ENAME header & auth
loop For each input
GraphQL->>DB: storeMetaEnvelopeWithId(meta, acl, id?)
DB-->>GraphQL: Created envelope with id
GraphQL->>Logging: appendEnvelopeOperationLog("create", ...)
Logging-->>GraphQL: Logged
alt skipWebhooks not set & authorized migration
GraphQL->>Webhooks: deliverWebhooks(webhookPayload)
Webhooks-->>GraphQL: Fire-and-forget with error logging
end
GraphQL->>GraphQL: Accumulate result { id, success/error }
end
GraphQL-->>Client: { results[], successCount, errorCount, errors[] }
sequenceDiagram
actor Migration as Migration Process
participant MigrationService
participant OldRegistry as Old eVault Registry
participant GraphQL as New eVault GraphQL API
participant Auth as Platform Auth
Migration->>MigrationService: copyMetaEnvelopes(oldUri, newUri)
MigrationService->>Auth: getPlatformToken()
Auth-->>MigrationService: platformToken
MigrationService->>OldRegistry: fetchAllMetaEnvelopes(paginated GraphQL)
OldRegistry-->>MigrationService: Envelope batches
loop For each batch
MigrationService->>GraphQL: bulkCreateOnNewEvault(envelopes, token)
GraphQL-->>MigrationService: { successCount, errorCount }
MigrationService->>MigrationService: Log batch progress & save state
end
MigrationService-->>Migration: Migration complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
coodos
left a comment
There was a problem hiding this comment.
need tests for new mutation + docs
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
infrastructure/evault-core/src/core/protocol/vault-access-guard.ts (1)
231-253:⚠️ Potential issue | 🟠 MajorPaginated MetaEnvelope connections bypass ACL filtering.
When a resolver returns a connection object (e.g.,
metaEnvelopes), the middleware doesn’t filteredges[].nodeby ACL—so any authenticated caller can enumerate all metaEnvelopes. Consider detecting connection shapes and filtering edges viafilterEnvelopesByAccess, or enforce ACL constraints in the DB query.
🤖 Fix all issues with AI agents
In `@infrastructure/evault-core/src/core/db/db.service.ts`:
- Around line 143-225: storeMetaEnvelopeWithId uses CREATE for the MetaEnvelope
(and currently creates Envelope nodes with CREATE), which causes duplicate-node
errors on retries; change the Cypher to use MERGE for the MetaEnvelope node
(MERGE (m:MetaEnvelope { id: $metaId })) and for each envelope node MERGE on the
id (MERGE (eX:Envelope { id: $eX_id })) and then use ON CREATE SET to assign
ontology, value, valueType and ON MATCH DO NOTHING (or avoid overwriting) so the
operation becomes idempotent; update the cypher strings built in
storeMetaEnvelopeWithId (referencing metaId, envelopeParams and the alias like
e0/e1) and keep using runQueryInternal to execute the merged query.
In `@infrastructure/evault-core/src/core/protocol/vault-access-guard.ts`:
- Around line 220-226: The isStoreOperation heuristic in vault-access-guard.ts
currently treats any args.input with ontology/payload/acl and no args.id as a
store operation (variable isStoreOperation), which inadvertently matches
createMetaEnvelope and bypasses Bearer-token checks; update the check to only
mark true for the actual storeMetaEnvelope call (either by checking the
resolver/method name passed into the guard or by using an explicit flag you set
when wrapping storeMetaEnvelope), e.g. refine isStoreOperation to also verify
the resolver name is "storeMetaEnvelope" or read a dedicated isStoreOperation
flag on the resolver context before skipping token auth so createMetaEnvelope
remains authenticated.
In `@platforms/emover-api/src/services/MigrationService.ts`:
- Around line 403-489: The bulkCreateOnNewEvault function currently sets acl:
["*"] which widens access; change it to preserve original ACLs by reading each
envelope's ACL (e.g., env.acl or env.parsed?.acl) and pass that into the inputs
for bulkCreateMetaEnvelopes, falling back to a safe default (e.g., empty or
existing eVault default) only when no ACL is present; if preservation requires
privileged transfer, accept/obtain a privileged token or flag and use that path
instead of forcing public ACLs (update references in bulkCreateOnNewEvault to
use the envelope ACL field and adjust callers if you add a
preserveAcl/privilegedToken parameter).
- Around line 317-401: fetchAllMetaEnvelopes currently queries the standard
GraphQL metaEnvelopes (via graphqlUrl, Authorization and X-ENAME headers) which
is subject to ACL filtering and can omit private envelopes; change the call in
fetchAllMetaEnvelopes to use a privileged migration export path or an
access-guard bypass that returns all envelopes (e.g. swap to a dedicated export
endpoint or include a certified migration token/header instead of the current
platform token), update the request headers/URL used in axios.post (replace
graphqlUrl/Authorization/X-ENAME usage) and ensure the server-side recognizes
and validates the migration credential so results are not ACL-filtered.
8c35f9b to
7f77ff7
Compare
Description of change
shifts emover to use the idiomatic graphql api
Issue Number
Closes #767
Type of change
How the change has been tested
manual
Change checklist
Summary by CodeRabbit
Release Notes
New Features
Refactor