Feat/awareness as a service#962
Merged
Merged
Conversation
Add the awareness-service workspace under services/ with the api package: TypeORM entities (Packet, Consumer, AccessApplication, ApiKey, Subscription, Delivery, DeadLetter), Postgres data-source, config loader, pagination cursor and backoff utilities, and an Express bootstrap with a health endpoint.
Add POST /ingest as the single sink for evault-core awareness packets, authenticated by a shared secret. IngestService upserts the packet and queues one delivery per matching subscription (idempotent via a unique constraint). SubscriptionMatcher resolves matches by ontology and eVault filters. The DeliveryEngine drains the queue with FOR UPDATE SKIP LOCKED batching, exponential backoff retries, optional HMAC signing, and a dead-letter table.
Add consumer-facing endpoints, all authenticated by issued API keys: - GET /api/packets: poll awareness history by ontology, eVault and time range with opaque (receivedAt, id) cursor pagination. - /api/subscriptions: register/update/remove webhook subscriptions filtered by ontology and eVault. - /api/me: consumer profile, API key rotation, and recent delivery status. Includes ApiKeyService (SHA-256 hashed, plaintext shown once) and the consumerAuth middleware.
Add the portal-facing surface: - /api/auth: W3DS login - offer a w3ds://auth deeplink, verify the wallet signature against the registry, exchange it for a session JWT. - /api/applications: a logged-in platform applies for access and tracks status. - /api/admin: whitelisted admins (AAAS_ADMIN_ENAMES) approve/reject applications and inspect/replay dead-lettered deliveries. consumerAuth now accepts either a machine API key or a portal session JWT so a freshly approved consumer can issue its first key from the dashboard.
Add the one-time migration path: - backfill-neo4j.ts reads MetaEnvelopes directly from evault-core's Neo4j (same node), reconstructs each packet's data payload and upserts into the packets table. Seeds history only - no deliveries are queued. - SeedService ensures every platform in the registry has an approved consumer and a catch-all subscription, so existing webhook receivers keep working unchanged. Runs on every API launch and via the seed:catchall script.
evault-core no longer queries the registry and fans out webhooks to every platform. getActivePlatforms and deliverWebhooks are removed; a single notifyAwareness POST forwards each awareness packet to AWARENESS_SERVICE_URL/ ingest, authenticated by a shared secret. All five mutation call sites (create, update, bulk-create, binding document create/sign) are updated. The requesting platform is passed through to AaaS so it can skip delivering a packet back to its origin, preserving the ping-pong guard the old fanout had.
Add the portal: W3DS QR login, an access application form, a consumer dashboard (API key rotation, webhook subscription management, recent delivery status), an admin queue to approve/reject applications, and a dead-letter view with replay. Mirrors the enotary SvelteKit + Tailwind stack.
- Add the initial TypeORM migration creating all AaaS tables and indexes. - Register services/*/* in the pnpm workspace so the api and portal packages resolve. - Add AaaS environment variables to .env.example. - Type jsonb columns as any so TypeORM's deep-partial insert types accept full packet payloads. - Add the service README.
Contributor
📝 WalkthroughWalkthroughThis PR introduces "Awareness as a Service (AaaS)," a new microservice that consolidates webhook delivery, subscription management, and consumer access control. It replaces the previous webhook fanout in evault-core with a centralized ingest endpoint, implements a background delivery engine with retry/dead-lettering, and provides a Svelte portal for consumer self-service and admin review. The PR includes a complete Express API backend, PostgreSQL schema, auth services, and integration changes to forward awareness packets from evault-core mutations. ChangesDatabase Schema, Configuration & Service Initialization
Ingest, Delivery & Retry Mechanisms
API Controllers, Auth Middleware & OpenAPI Documentation
Portal Frontend: Svelte Pages and Auth State
Integration & Migration: evault-core to AaaS
🎯 4 (Complex) | ⏱️ ~45 minutes
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![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: 6
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
🟠 Major comments (27)
services/awareness-service/api/src/scripts/backfill-neo4j.ts-39-41 (1)
39-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail fast when Neo4j env is missing instead of using implicit credentials.
Lines 39-41 default to
bolt://localhost:7687andneo4j/neo4j; this can seed from the wrong graph silently in migration contexts.Suggested fix
-const uri = process.env.NEO4J_URI ?? "bolt://localhost:7687"; -const user = process.env.NEO4J_USER ?? "neo4j"; -const password = process.env.NEO4J_PASSWORD ?? "neo4j"; +const uri = process.env.NEO4J_URI; +const user = process.env.NEO4J_USER; +const password = process.env.NEO4J_PASSWORD; +if (!uri || !user || !password) { + throw new Error("Missing required Neo4j env: NEO4J_URI, NEO4J_USER, NEO4J_PASSWORD"); +}🤖 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 `@services/awareness-service/api/src/scripts/backfill-neo4j.ts` around lines 39 - 41, Remove the implicit defaults and make connection parameters fail fast: stop using the fallback values for process.env.NEO4J_URI, process.env.NEO4J_USER, and process.env.NEO4J_PASSWORD in backfill-neo4j.ts (the uri, user, password bindings), validate that each env var is present at startup, and throw a clear error (including which variable is missing) if any are absent so the script will not silently connect to localhost/neo4j credentials.services/awareness-service/api/src/services/SeedService.ts-29-32 (1)
29-32:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not silently succeed when registry fetch fails.
Lines 29-32 convert a hard dependency failure into
{ seeded: 0, total: 0 }, which can hide broken backward-compat seeding at startup.Suggested fix
} catch (err) { console.error("[seed] failed to fetch registry platforms:", err); - return { seeded: 0, total: 0 }; + throw new Error("Catch-all seeding aborted: unable to fetch registry platforms"); }🤖 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 `@services/awareness-service/api/src/services/SeedService.ts` around lines 29 - 32, The catch block that currently logs "[seed] failed to fetch registry platforms" and returns { seeded: 0, total: 0 } should not convert a hard dependency failure into a silent success; change the handler in SeedService.ts so that it surfaces the failure (either rethrow the caught error or return an explicit error/result object) instead of returning a zeroed success value—locate the catch around the registry fetch (the block logging "[seed] failed to fetch registry platforms") and replace the return with throwing the original err (or return a clearly failing result that callers check) so startup seeding failures are visible to callers and CI.services/awareness-service/api/src/scripts/backfill-neo4j.ts-58-62 (1)
58-62:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd deterministic ordering to paginated Neo4j reads.
Line 61 paginates with
SKIP/LIMITbut noORDER BY; this can make backfill coverage non-deterministic and miss records across batches.Suggested fix
const result = await session.run( `MATCH (m:MetaEnvelope)-[:LINKS_TO]->(e:Envelope) RETURN m.id AS id, m.ontology AS ontology, m.eName AS eName, collect({ontology: e.ontology, value: e.value, valueType: e.valueType}) AS envelopes + ORDER BY id SKIP $skip LIMIT $batch`, { skip: neo4j.int(skip), batch: neo4j.int(BATCH) }, );🤖 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 `@services/awareness-service/api/src/scripts/backfill-neo4j.ts` around lines 58 - 62, The paginated Neo4j query using SKIP/LIMIT in the query string (`MATCH (m:MetaEnvelope)-[:LINKS_TO]->(e:Envelope) ... SKIP $skip LIMIT $batch`) lacks an ORDER BY, making pagination non-deterministic; update the Cypher to include a deterministic ordering (e.g., append `ORDER BY m.id ASC` or `ORDER BY m.eName ASC, m.id ASC` before `SKIP $skip LIMIT $batch`) so the `skip`/`limit` pages are stable, and ensure the same ordering key is used consistently across all batched reads in this backfill code.services/awareness-service/api/src/database/entities/Subscription.ts-41-43 (1)
41-43:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid persisting webhook signing secrets in plaintext.
This stores material that can be used to forge signed callbacks if DB reads/backups are exposed. Prefer encrypted-at-rest application-level storage (envelope encryption / KMS) and decrypt only at delivery time.
🤖 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 `@services/awareness-service/api/src/database/entities/Subscription.ts` around lines 41 - 43, The Subscription entity currently persists webhook signing material in plaintext via the secret field (`@Column` secret), so replace it with a secure storage pattern: remove/avoid storing raw secret and instead store either an encrypted column (e.g., encryptedSecret) using a TypeORM ValueTransformer that encrypts/decrypts with your KMS/local envelope key, or store a reference keyId/secretId that points to the secret in a KMS/secret store and fetch/decrypt at delivery time; update any code that reads Subscription.secret (e.g., delivery path) to instead call the decrypt/fetch routine just-in-time so plaintext signing secrets are never stored in the DB.services/awareness-service/api/src/database/migrations/1715200000000-Init.ts-62-155 (1)
62-155:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd foreign keys for core ownership and delivery-chain relations.
The schema defines relationship IDs but no FK constraints. That allows orphaned
api_keys,subscriptions,deliveries, anddead_letters, which can break replay/delivery assumptions and data integrity.🧩 Suggested constraint additions
+await queryRunner.query(` + ALTER TABLE "access_applications" + ADD CONSTRAINT "fk_access_applications_consumer" + FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE +`); + +await queryRunner.query(` + ALTER TABLE "api_keys" + ADD CONSTRAINT "fk_api_keys_consumer" + FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE +`); + +await queryRunner.query(` + ALTER TABLE "subscriptions" + ADD CONSTRAINT "fk_subscriptions_consumer" + FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE +`); + +await queryRunner.query(` + ALTER TABLE "deliveries" + ADD CONSTRAINT "fk_deliveries_subscription" + FOREIGN KEY ("subscriptionId") REFERENCES "subscriptions"("id") ON DELETE CASCADE +`); + +await queryRunner.query(` + ALTER TABLE "deliveries" + ADD CONSTRAINT "fk_deliveries_packet" + FOREIGN KEY ("packetId") REFERENCES "packets"("id") ON DELETE CASCADE +`); + +await queryRunner.query(` + ALTER TABLE "dead_letters" + ADD CONSTRAINT "fk_dead_letters_delivery" + FOREIGN KEY ("deliveryId") REFERENCES "deliveries"("id") ON DELETE CASCADE +`);🤖 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 `@services/awareness-service/api/src/database/migrations/1715200000000-Init.ts` around lines 62 - 155, The migration creates owner and chain identifier columns but omits FK constraints, allowing orphans; add foreign keys: ALTER TABLE "access_applications" ADD CONSTRAINT fk_access_applications_consumer FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE; for "api_keys" add fk_api_keys_consumer FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE; for "subscriptions" add fk_subscriptions_consumer FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE; for "deliveries" add fk_deliveries_subscription FOREIGN KEY ("subscriptionId") REFERENCES "subscriptions"("id") ON DELETE CASCADE; and for "dead_letters" add fk_dead_letters_delivery FOREIGN KEY ("deliveryId") REFERENCES "deliveries"("id") ON DELETE CASCADE, fk_dead_letters_subscription FOREIGN KEY ("subscriptionId") REFERENCES "subscriptions"("id") ON DELETE CASCADE, and fk_dead_letters_consumer FOREIGN KEY ("consumerId") REFERENCES "consumers"("id") ON DELETE CASCADE; include these ALTER TABLE statements in the same migration (or equivalent CREATE TABLE ... REFERENCES clauses) and add matching DROP CONSTRAINTs in the down migration.services/awareness-service/api/src/database/data-source.ts-28-30 (1)
28-30:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSSL certificate validation is disabled.
Setting
rejectUnauthorized: falsedisables certificate validation, making the connection vulnerable to man-in-the-middle attacks even when using SSL. This setting should only be used in development with self-signed certificates.🔒 Safer approach
Consider making this configurable or at minimum documenting the security tradeoff:
ssl: config.dbCaCert - ? { rejectUnauthorized: false, ca: config.dbCaCert } + ? { ca: config.dbCaCert } : false,If you must support self-signed certificates, add a separate environment flag:
ssl: config.dbCaCert ? { ca: config.dbCaCert, rejectUnauthorized: process.env.DB_REJECT_UNAUTHORIZED !== 'false' } : 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 `@services/awareness-service/api/src/database/data-source.ts` around lines 28 - 30, The SSL config currently sets rejectUnauthorized: false when config.dbCaCert is present; change this to use a configurable flag so certificate validation remains enabled by default. Update the ssl assignment that references config.dbCaCert to set ca: config.dbCaCert and set rejectUnauthorized based on a new environment/config flag (e.g., DB_REJECT_UNAUTHORIZED or config.dbRejectUnauthorized) so it defaults to true and can only be disabled explicitly for self-signed certs; adjust any callers/initialization that build the data source to read that flag and ensure config.dbCaCert and rejectUnauthorized are used together in the ssl object.services/awareness-service/api/src/index.ts-23-23 (1)
23-23:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestrict CORS origins for production.
cors()with no options allows all origins, which may expose the API to cross-origin attacks. Configure allowed origins explicitly.🔒 Proposed fix
- app.use(cors()); + app.use(cors({ + origin: process.env.ALLOWED_ORIGINS?.split(',') ?? '*', + credentials: true + }));Then add to
.env.example:ALLOWED_ORIGINS=http://localhost:3000,https://portal.example.com🤖 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 `@services/awareness-service/api/src/index.ts` at line 23, The app currently enables open CORS via app.use(cors())—change it to read ALLOWED_ORIGINS from env, parse the comma-separated list, and pass it as the origin option to cors() (e.g., app.use(cors({ origin: allowedOrigins }))); ensure a sensible default for local dev if ALLOWED_ORIGINS is missing and validate entries before passing them to cors; update .env.example to include ALLOWED_ORIGINS (e.g., ALLOWED_ORIGINS=http://localhost:3000,https://portal.example.com).services/awareness-service/api/src/config.ts-16-16 (1)
16-16:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse
required()for critical database configuration.The
databaseUrldefaults to an empty string, which will cause a late runtime failure when TypeORM attempts to connect. Use therequired()helper to fail fast at startup.🛡️ Proposed fix
- databaseUrl: process.env.AWARENESS_DATABASE_URL ?? "", + databaseUrl: required("AWARENESS_DATABASE_URL"),🤖 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 `@services/awareness-service/api/src/config.ts` at line 16, The config currently sets databaseUrl to process.env.AWARENESS_DATABASE_URL ?? "" which hides missing config and leads to runtime TypeORM failures; update the databaseUrl assignment in config.ts to use the required() helper for the AWARENESS_DATABASE_URL env var (so the application fails fast at startup with a clear error) — modify the databaseUrl export/variable in config.ts to call required(...) instead of defaulting to an empty string.services/awareness-service/api/src/index.ts-58-66 (1)
58-66: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winAdd graceful shutdown handling.
The server does not handle
SIGTERMorSIGINTsignals, which means the database connection, delivery engine, and in-flight requests are not cleaned up on shutdown. This can lead to resource leaks or orphaned deliveries.♻️ Proposed graceful shutdown handler
app.listen(config.apiPort, () => { console.log(`[aaas] API listening on :${config.apiPort}`); }); + + // Graceful shutdown + const shutdown = async (signal: string) => { + console.log(`[aaas] ${signal} received, shutting down gracefully...`); + deliveryEngine.stop(); + await AppDataSource.destroy(); + process.exit(0); + }; + + process.on('SIGTERM', () => shutdown('SIGTERM')); + process.on('SIGINT', () => shutdown('SIGINT')); }🤖 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 `@services/awareness-service/api/src/index.ts` around lines 58 - 66, Add graceful shutdown by capturing the Server returned from app.listen (assign to e.g., "server"), and in the start() flow register SIGINT and SIGTERM handlers that (1) stop accepting new requests via server.close(), (2) await shutdown/cleanup of the delivery engine (e.g., deliveryEngine.stop() or equivalent), (3) close the database connection (e.g., dbClient.close()/disconnect()), and (4) then exit the process with appropriate code; ensure the handlers are idempotent and log errors, and wire this cleanup into the existing start() error path so failures also trigger the same cleanup sequence.services/awareness-service/api/src/config.ts-29-29 (1)
29-29:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove default JWT secret for production safety.
The hardcoded default
"awareness-dev-secret"is predictable and allows session forgery in any deployment whereAAAS_JWT_SECRETis not explicitly set.🔒 Proposed fix
- jwtSecret: process.env.AAAS_JWT_SECRET ?? "awareness-dev-secret", + jwtSecret: required("AAAS_JWT_SECRET"),🤖 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 `@services/awareness-service/api/src/config.ts` at line 29, The config entry jwtSecret currently falls back to a hardcoded default ("awareness-dev-secret"); remove this insecure default and require a real secret from AAAS_JWT_SECRET. Update the code that defines jwtSecret to read process.env.AAAS_JWT_SECRET and fail fast (throw an error or exit) when it's undefined or empty so the service will not start without an explicit secret; reference the jwtSecret variable and the AAAS_JWT_SECRET environment variable in config.ts when implementing this check.services/awareness-service/api/src/index.ts-51-53 (1)
51-53:⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffAdd unique constraint on Subscription(consumerId, isCatchAll, targetUrl) to ensure true idempotency.
The JSDoc claims the seeding is idempotent, but the Subscription creation is not atomic. While the Consumer creation is protected by the unique index on
ename, the Subscription check-then-create pattern (lines 62–79) has a race condition window. Two concurrent instances could both pass thefindOne()check before either executessave(), resulting in duplicate catch-all subscriptions. Add a unique database constraint on(consumerId, isCatchAll, targetUrl)to prevent this.🤖 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 `@services/awareness-service/api/src/index.ts` around lines 51 - 53, Add a DB-level unique constraint on Subscription(consumerId, isCatchAll, targetUrl) to guarantee idempotency for SeedService.seedCatchAll; update the Subscription schema/model to declare a unique composite index on consumerId + isCatchAll + targetUrl, add a migration to create that unique index (and optionally clean up existing dupes before applying), and ensure seedCatchAll’s create logic handles unique-constraint errors (treat as success) so concurrent seeders can’t insert duplicates.services/awareness-service/portal/src/lib/session.ts-7-16 (1)
7-16:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftAvoid persisting the portal JWT in
localStorage.Keeping the bearer session token in
localStoragematerially increases blast radius for any XSS path (including admin-capable sessions). Prefer HttpOnly, Secure, SameSite cookies with server-side verification.🤖 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 `@services/awareness-service/portal/src/lib/session.ts` around lines 7 - 16, The current sessionToken writable (sessionToken, STORAGE_KEY) and its subscribe side-effect persist the JWT to localStorage; remove that client-side persistence and the localStorage reads/writes in session.ts and instead rely on server-set HttpOnly, Secure, SameSite cookies for the bearer token; initialize sessionToken from server-provided state (SSR/hydration or an auth endpoint) rather than localStorage, delete the subscribe callback that calls localStorage.setItem/removeItem, and ensure any code that reads STORAGE_KEY or expects localStorage is updated to obtain the token via cookie-aware server calls or a secure authentication API.services/awareness-service/portal/src/routes/admin/+page.svelte-92-99 (1)
92-99:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate webhook URLs before rendering as links.
webhookBaseUrlis applicant-controlled data. Rendering it directly ashrefallows dangerous schemes (e.g.javascript:), which can execute when an admin clicks.Proposed patch
<script lang="ts"> @@ + function safeHttpUrl(value: string): string | null { + try { + const u = new URL(value); + return u.protocol === "http:" || u.protocol === "https:" ? u.toString() : null; + } catch { + return null; + } + } </script> @@ - {`#if` app.consumer?.webhookBaseUrl} + {`#if` app.consumer?.webhookBaseUrl && + safeHttpUrl(app.consumer.webhookBaseUrl)} <a - href={app.consumer.webhookBaseUrl} + href={safeHttpUrl(app.consumer.webhookBaseUrl)} target="_blank" rel="noopener" class="text-sm text-indigo-400 hover:underline" > {app.consumer.webhookBaseUrl} </a> {/if}🤖 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 `@services/awareness-service/portal/src/routes/admin/`+page.svelte around lines 92 - 99, The link uses untrusted applicant data (app.consumer.webhookBaseUrl) directly as an href; change this to validate and sanitize before rendering: parse the value (e.g., via URL constructor) and only allow http: or https: protocols, setting a safe variable like safeWebhookHref for the anchor; if parsing fails or protocol is not http/https, do not render an actionable <a> (render escaped plain text or a non-clickable element) and ensure you never insert the raw string into href. Locate uses of app.consumer.webhookBaseUrl in the component and replace them with the validated/sanitized safe value and fallback rendering logic.services/awareness-service/portal/src/routes/+layout.svelte-23-28 (1)
23-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRedirect immediately after logout.
Right now logout clears token state but leaves the current protected page rendered. Redirecting to
/on click prevents stale sensitive content from lingering in view.Proposed patch
<script lang="ts"> import "../app.css"; + import { goto } from "$app/navigation"; import { sessionToken, session, logout } from "$lib/session"; @@ - <button + <button class="text-sm text-gray-400 hover:text-white" - onclick={logout} + onclick={() => { + logout(); + goto("/"); + }} > Log out </button>🤖 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 `@services/awareness-service/portal/src/routes/`+layout.svelte around lines 23 - 28, The logout handler currently clears token state but does not navigate away, so update the logout flow: in the logout function referenced by the onclick on the Log out button, after clearing auth/token state call a navigation redirect to the root (e.g., use SvelteKit's goto('/') or window.location.assign('/') depending on your router) so the protected page is immediately replaced; ensure the onclick remains bound to logout and perform the redirect as the final step in the logout function.services/awareness-service/api/src/controllers/IngestController.ts-16-20 (1)
16-20:⚠️ Potential issue | 🟠 Major | ⚡ Quick winIngest auth is fail-open when
config.ingestSecretis missing.If
config.ingestSecretis empty, anyone can POST to/ingest. This should fail closed.Suggested change
- if ( - config.ingestSecret && - req.header("x-ingest-secret") !== config.ingestSecret - ) { + if (!config.ingestSecret) { + return res.status(500).json({ error: "ingest auth not configured" }); + } + if (req.header("x-ingest-secret") !== config.ingestSecret) { return res.status(401).json({ error: "invalid ingest secret" }); }🤖 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 `@services/awareness-service/api/src/controllers/IngestController.ts` around lines 16 - 20, The current auth check in IngestController (the block using config.ingestSecret and req.header("x-ingest-secret")) is fail-open when config.ingestSecret is falsy; change the logic so that if config.ingestSecret is missing or the header doesn't match, the request is rejected (e.g., return res.status(401).json({ error: "invalid ingest secret" })); ensure the check explicitly verifies presence of config.ingestSecret before allowing any requests and denies when it is empty or undefined.infrastructure/evault-core/src/core/protocol/graphql-server.ts-85-88 (1)
85-88:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid logging full webhook payload content in production logs.
This currently logs
payload=${payloadJson}, which can expose sensitive packet data (data) and user identifiers in logs. Log stable metadata only (id/schema/w3id/size) instead.Suggested change
- const payloadJson = JSON.stringify(webhookPayload); + const payloadJson = JSON.stringify(webhookPayload); console.log( - `[webhook] id=${webhookPayload?.id} schemaId=${webhookPayload?.schemaId} w3id=${webhookPayload?.w3id} payload=${payloadJson}`, + `[webhook] id=${webhookPayload?.id} schemaId=${webhookPayload?.schemaId} w3id=${webhookPayload?.w3id} payloadBytes=${payloadJson.length}`, );🤖 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 `@infrastructure/evault-core/src/core/protocol/graphql-server.ts` around lines 85 - 88, The current console.log prints the entire webhook payload via payloadJson which can leak sensitive data; update the logging in graphql-server.ts (the console.log that references webhookPayload and payloadJson) to omit the full payload and instead log only stable metadata: webhookPayload.id, webhookPayload.schemaId, webhookPayload.w3id and the payload size (e.g., compute size from payloadJson.length or Buffer.byteLength(payloadJson)) so logs contain id/schemaId/w3id/size but not payload contents.services/awareness-service/api/src/services/DeliveryEngine.ts-158-163 (1)
158-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrong fallback value for
DeadLetter.consumerId.
delivery.subscriptionIdis not a consumer id. Using it as fallback corrupts dead-letter attribution and may violate FK constraints.Suggested change
- consumerId: subscription?.consumerId ?? delivery.subscriptionId, + consumerId: subscription?.consumerId ?? null,🤖 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 `@services/awareness-service/api/src/services/DeliveryEngine.ts` around lines 158 - 163, The DeadLetter insert is using delivery.subscriptionId as a fallback for consumerId which is incorrect; update the insertion in DeliveryEngine (the insert call creating DeadLetter) to set consumerId to subscription?.consumerId ?? delivery.consumerId ?? null (or another valid consumer identifier) instead of delivery.subscriptionId, and ensure the value conforms to the DeadLetter FK (throw or skip insert if no valid consumerId) so dead-letter attribution and FK constraints remain correct.services/awareness-service/api/src/middleware/consumerAuth.ts-34-52 (1)
34-52:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftPreserve the credential type for downstream authorization.
This middleware collapses API keys and portal JWTs into the same
req.consumershape. That means routes likePOST /api/me/api-keyscannot require an interactive portal login, so any leaked machine key can mint replacement keys and persist access.Also applies to: 59-60
🤖 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 `@services/awareness-service/api/src/middleware/consumerAuth.ts` around lines 34 - 52, The middleware currently normalizes both API keys and portal JWTs into the same req.consumer, losing credential provenance; update the consumerAuth middleware to attach credential type and original credential object to the request (e.g., set req.authType = "api_key" or "portal_jwt" and set req.authCredential = apiKey or decoded) alongside req.consumer so downstream handlers (like POST /api/me/api-keys) can require interactive portal_jwt; apply the same change for the other branch referenced at lines 59-60 and ensure names used match existing symbols (token, apiKey, decoded, consumerRepo, consumer).services/awareness-service/api/src/services/W3dsAuthService.ts-24-25 (1)
24-25:⚠️ Potential issue | 🟠 Major | ⚡ Quick winEnforce the session TTL on callback and poll too.
SESSION_TTL_MSis only applied when a new offer is created. If no latercreateOffer()runs, an old session can still be completed and exchanged for a JWT long after the intended 10-minute window.🔒 Minimal fix
async completeLogin( ename: string, session: string, signature: string, ): Promise<{ ok: boolean; error?: string }> { + this.gc(); const pending = this.sessions.get(session); if (!pending) return { ok: false, error: "unknown or expired session" }; @@ pollSession( session: string, ): { status: "pending" } | { status: "authenticated"; token: string } { + this.gc(); const pending = this.sessions.get(session); if (!pending) { return { status: "pending" }; }Also applies to: 36-42, 63-67, 101-105
🤖 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 `@services/awareness-service/api/src/services/W3dsAuthService.ts` around lines 24 - 25, The session TTL (SESSION_TTL_MS) is only enforced when createOffer() runs, so callback and poll handlers can complete stale sessions; update W3dsAuthService to store a creation timestamp with each session (when randomUUID() is created in createOffer()), run gc() as now, and then in the callback handler and the poll handler (the methods handling the callback and polling logic) check the stored timestamp against SESSION_TTL_MS and reject/cleanup any session that has expired before proceeding to exchange or return a JWT; ensure expired sessions are removed from the session store and that these handlers return an appropriate error when TTL has lapsed.services/awareness-service/api/src/services/W3dsAuthService.ts-20-20 (1)
20-20:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMove login session state out of process memory.
This flow depends on a singleton
Map, so any restart or multi-instance deployment breaks the offer → callback → poll handshake. Users will get stuck with “pending” whenever those three requests land on different processes.Also applies to: 109-110
🤖 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 `@services/awareness-service/api/src/services/W3dsAuthService.ts` at line 20, The current W3dsAuthService stores login handshake state in an in-memory Map named sessions (type PendingSession), which breaks across restarts and multi-instance deployments; replace the in-process Map with a shared persistent store (e.g., Redis or your existing DB) and implement session CRUD methods used by the service (createPendingSession, getPendingSession, deletePendingSession) that serialize/deserialize PendingSession so login, callback and poll flows all use the shared store instead of the private sessions Map; update any references to this.sessions (including where PendingSession is created at login, looked up in the callback handler, and polled) to call the new store-backed methods so state survives restarts and works across instances.services/awareness-service/api/src/services/ApiKeyService.ts-37-41 (1)
37-41:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle rejection from the fire-and-forget
lastUsedAtupdate.
void ...update(...)still leaves a rejected promise unhandled. A transient failure in best-effort usage tracking should not bubble into process-level rejection handling.🩹 Localized fix
if (apiKey) { // best-effort last-used tracking - void AppDataSource.getRepository(ApiKey).update(apiKey.id, { - lastUsedAt: new Date(), - }); + void AppDataSource.getRepository(ApiKey) + .update(apiKey.id, { + lastUsedAt: new Date(), + }) + .catch(() => undefined); }🤖 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 `@services/awareness-service/api/src/services/ApiKeyService.ts` around lines 37 - 41, The fire-and-forget update to track lastUsedAt currently ignores rejections and can produce unhandled promise rejections; wrap the AppDataSource.getRepository(ApiKey).update(...) call inside a promise chain or async wrapper that catches errors (e.g., Promise.resolve(...).catch(...) or an immediately-invoked async function) and handle failures by logging them (using the existing logger) or swallowing them explicitly so they don't propagate; update the code in ApiKeyService where the void AppDataSource.getRepository(ApiKey).update(apiKey.id, { lastUsedAt: new Date() }) call occurs to attach a .catch(...) handler that logs/debugs the error and returns void.services/awareness-service/api/src/controllers/ConsumerController.ts-68-71 (1)
68-71:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp
limitto a positive range.Negative values pass straight through here, so
?limit=-1can turn into an invalid SQLLIMITor other undefined paging behavior instead of falling back to the default.📏 Tight fix
router.get("/api/me/deliveries", async (req, res) => { - const limit = Math.min( - parseInt(String(req.query.limit ?? "50"), 10) || 50, - 200, - ); + const parsedLimit = Number.parseInt( + String(req.query.limit ?? "50"), + 10, + ); + const limit = Number.isFinite(parsedLimit) + ? Math.min(Math.max(parsedLimit, 1), 200) + : 50; const deliveries = await AppDataSource.getRepository(Delivery)🤖 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 `@services/awareness-service/api/src/controllers/ConsumerController.ts` around lines 68 - 71, The limit calculation in ConsumerController.ts allows negative values through; when parsing req.query.limit, ensure the parsed value is a positive integer before clamping to 200 and defaulting to 50: parse the incoming limit from req.query.limit, coerce NaN to the default (50), then clamp using Math.max(1, parsedValue) and Math.min(..., 200) so negative numbers become the default/minimum rather than propagating into SQL LIMIT; update the logic around the limit variable assignment (the parseInt + Math.min expression) to enforce this positive-range constraint.services/awareness-service/api/src/controllers/AdminController.ts-36-74 (1)
36-74:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap approve/reject state transitions in a single transaction.
Line 43–52 and Line 63–72 update
AccessApplicationandConsumerindependently. If the second write fails, admin review state and consumer status diverge.🤖 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 `@services/awareness-service/api/src/controllers/AdminController.ts` around lines 36 - 74, The approve/reject handlers currently update AccessApplication and Consumer in two separate writes which can diverge on failure; wrap the state transitions for application.status/reviewedByEname/reviewNote/reviewedAt and the Consumer.update in a single DB transaction using AppDataSource.manager.transaction (or AppDataSource.transaction) so both saves occur atomically; locate the router.post handlers for "/api/admin/applications/:id/approve" and "/api/admin/applications/:id/reject", fetch the AccessApplication and then perform application.save and Consumer.update inside the same transaction callback, returning appropriate errors if the transaction fails.services/awareness-service/api/src/openapi.ts-691-717 (1)
691-717:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAllow
nullfor/api/applications/meresponse members.The implementation can return
{ consumer: null, application: null }, but Line 704–709 currently references non-null schemas only. This misstates actual API behavior.Suggested spec patch
consumer: { - $ref: "`#/components/schemas/Consumer`", + oneOf: [ + { $ref: "`#/components/schemas/Consumer`" }, + { type: "null" }, + ], }, application: { - $ref: "`#/components/schemas/AccessApplication`", + oneOf: [ + { $ref: "`#/components/schemas/AccessApplication`" }, + { type: "null" }, + ], },🤖 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 `@services/awareness-service/api/src/openapi.ts` around lines 691 - 717, The /api/applications/me response schema currently requires "consumer" and "application" objects but the implementation may return null for either; update the response schema for the GET operation at "/api/applications/me" so the properties "consumer" and "application" accept null (e.g., set nullable: true on each property or use oneOf: [{ $ref: "`#/components/schemas/Consumer`" }, { type: "null" }] and similarly for AccessApplication) so the OpenAPI spec matches runtime behavior and tools will allow { consumer: null, application: null }.services/awareness-service/api/src/controllers/AdminController.ts-96-107 (1)
96-107:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t resolve dead letters unless delivery reset actually succeeds.
At Line 96–107, the code marks
deadLetter.resolved = trueunconditionally. IfDelivery.update(...)affects 0 rows, the dead letter is incorrectly resolved and effectively dropped.Suggested fix
- await AppDataSource.getRepository(Delivery).update( + const result = await AppDataSource.getRepository(Delivery).update( deadLetter.deliveryId, { status: "pending", attempts: 0, nextAttemptAt: new Date(), lastError: null, lastResponseStatus: null, }, ); + if (!result.affected) { + return res.status(404).json({ error: "delivery not found" }); + } deadLetter.resolved = true; await dlRepo.save(deadLetter);🤖 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 `@services/awareness-service/api/src/controllers/AdminController.ts` around lines 96 - 107, The code currently sets deadLetter.resolved = true and saves it regardless of whether the Delivery reset actually succeeded; change the flow to inspect the result of AppDataSource.getRepository(Delivery).update(...) (e.g., the UpdateResult.affected count) and only set deadLetter.resolved = true and call dlRepo.save(deadLetter) when the update affected at least one row; if affected is 0, do not mark resolved and instead return/throw or log an error so the dead letter is not dropped (refer to the Delivery update call, deadLetter.resolved, and dlRepo.save to locate the logic).services/awareness-service/api/src/openapi.ts-719-771 (1)
719-771:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix
/api/applicationsrequest schema to match the implemented payload.Line 731–738 documents
contactEmail/webhookBaseUrl/justification/requestedOntologies, but the handler expectsname/websiteUrl/descriptionand rejects missing fields. This is a breaking contract mismatch for generated clients.Suggested spec patch
requestBody: { required: true, content: { "application/json": { schema: { type: "object", properties: { name: { type: "string" }, - contactEmail: { type: "string" }, - webhookBaseUrl: { type: "string" }, - justification: { type: "string" }, - requestedOntologies: { - type: "array", - items: { type: "string" }, - }, + websiteUrl: { type: "string" }, + description: { type: "string" }, }, + required: ["name", "websiteUrl", "description"], }, }, }, }, responses: { + "400": { $ref: "`#/components/responses/BadRequest`" }, "201": {🤖 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 `@services/awareness-service/api/src/openapi.ts` around lines 719 - 771, The OpenAPI requestBody schema for the "/api/applications" path is incorrect: update the schema properties to match the handler's expected payload (replace contactEmail/webhookBaseUrl/justification/requestedOntologies with name, websiteUrl, and description), mark the handler-required fields as required in the schema, and ensure the content type remains "application/json" so generated clients send the correct shape; locate and edit the requestBody -> content -> "application/json" -> schema object under the "/api/applications" POST operation to make these changes.services/awareness-service/api/src/controllers/ApplicationController.ts-30-74 (1)
30-74:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftMake the application write path atomic (consumer + application).
From Line 34 through Line 73, the read-then-write flow is non-transactional. Concurrent requests can both miss the pending row check (Line 64) and create duplicates, and failures after
consumerRepo.save(Line 61) can leave partial state.Suggested direction
router.post("/api/applications", async (req, res) => { - const consumerRepo = AppDataSource.getRepository(Consumer); - const appRepo = AppDataSource.getRepository(AccessApplication); + await AppDataSource.transaction(async (tx) => { + const consumerRepo = tx.getRepository(Consumer); + const appRepo = tx.getRepository(AccessApplication); // ... existing validation let consumer = await consumerRepo.findOne({ where: { ename: req.ename! }, + lock: { mode: "pessimistic_write" }, }); // ... create/update consumer let application = await appRepo.findOne({ where: { consumerId: consumer.id, status: "pending" }, + lock: { mode: "pessimistic_write" }, }); // ... create/update application - res.status(201).json({ consumer, application }); + res.status(201).json({ consumer, application }); + }); });🤖 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 `@services/awareness-service/api/src/controllers/ApplicationController.ts` around lines 30 - 74, The create-application flow in the router.post handler is non-atomic: concurrent requests can race on consumer/app lookups and you can end up with duplicate pending applications or half-saved consumer state; wrap the read-then-write logic (the consumerRepo.findOne, consumerRepo.create/save, appRepo.findOne/create/save and the assignment of consumer/application fields) inside a single database transaction (using AppDataSource.manager.transaction or a QueryRunner) so the pending-check and subsequent creates/updates are executed with proper locking/atomicity and any failure rolls back both consumer and application changes.
🧹 Nitpick comments (5)
services/awareness-service/api/src/database/migrations/1715200000000-Init.ts (1)
121-123: ⚡ Quick winOptimize queue claiming with a status-aware index.
If claims filter by status + due time, indexing only
nextAttemptAtwill degrade as delivered rows grow. Add a composite (or partial) index keyed by status and schedule time.⚡ Suggested index change
-`CREATE INDEX "idx_deliveries_next_attempt" ON "deliveries" ("nextAttemptAt")`, +`CREATE INDEX "idx_deliveries_status_next_attempt" ON "deliveries" ("status", "nextAttemptAt")`,Also applies to: 136-137
🤖 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 `@services/awareness-service/api/src/database/migrations/1715200000000-Init.ts` around lines 121 - 123, Add a composite/partial index to the migration so queue claiming queries that filter by "status" and "nextAttemptAt" use an index; update the migration's up method in 1715200000000-Init.ts (the Init migration) to create either a composite index on ("status","nextAttemptAt") or a partial index like ON ("nextAttemptAt") WHERE status = 'pending' and mirror removal in down; target the "status" and "nextAttemptAt" columns referenced in the migration so claims that filter by status + due time are covered by the index.services/awareness-service/portal/src/routes/admin/dead-letters/+page.svelte (2)
44-50: ⚡ Quick winGuard replay action against duplicate submissions.
Rapid clicks on “Replay” can fire multiple POSTs for the same item. Add per-item in-flight state and disable the button until completion.
Suggested patch
let deadLetters = $state<DeadLetter[]>([]); let error = $state<string | null>(null); let loading = $state(true); let notAdmin = $state(false); +let replaying = $state<Record<string, boolean>>({}); @@ async function replay(id: string) { + if (replaying[id]) return; + replaying[id] = true; try { await api(`/api/admin/dead-letters/${id}/replay`, { method: "POST", token: token(), }); await load(); } catch (e) { error = e instanceof Error ? e.message : "replay failed"; + } finally { + replaying[id] = false; } } @@ {:else} <button class="rounded bg-indigo-600 px-3 py-1.5 text-white hover:bg-indigo-500" + disabled={replaying[dl.id]} onclick={() => replay(dl.id)} > - Replay + {replaying[dl.id] ? "Replaying…" : "Replay"} </button> {/if}Also applies to: 97-100
🤖 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 `@services/awareness-service/portal/src/routes/admin/dead-letters/`+page.svelte around lines 44 - 50, The replay handler (async function replay(id: string)) must be guarded against duplicate submissions by tracking per-item in-flight state: add a map or Set (e.g., replayInFlight) keyed by dead-letter id, return early if the id is already in-flight, mark the id as in-flight before calling api(`/api/admin/dead-letters/${id}/replay`), and clear it in finally so it always gets reset; update the UI button for that item to be disabled when replayInFlight.has(id) (same pattern for the other replay handler referenced at 97-100), and ensure token() and load() calls remain unchanged inside the guarded block.
27-38: ⚡ Quick winMake admin-state detection deterministic instead of message-based.
Line 37 uses
msg.includes("admin")to setnotAdmin, which is fragile and can misclassify errors. Prefer structured status/code handling (e.g., 403), with message checks only as fallback.Suggested patch
async function load() { loading = true; error = null; + notAdmin = false; try { const result = await api<{ deadLetters: DeadLetter[] }>( "/api/admin/dead-letters", { token: token() }, ); deadLetters = result.deadLetters; } catch (e) { const msg = e instanceof Error ? e.message : "failed to load"; - if (msg.includes("admin")) notAdmin = true; + const status = + typeof e === "object" && e !== null && "status" in e + ? Number((e as { status?: unknown }).status) + : undefined; + if (status === 403 || /admin/i.test(msg)) notAdmin = true; else error = msg; } finally { loading = 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 `@services/awareness-service/portal/src/routes/admin/dead-letters/`+page.svelte around lines 27 - 38, The admin-state detection is currently fragile because it inspects the error text (msg.includes("admin")); instead, make it deterministic by checking a structured status/code from the API call: update the error handling around the api<{ deadLetters: DeadLetter[] }>(...) call to inspect a numeric HTTP status (e.g., error.status === 403 or error.code === 'FORBIDDEN') and set notAdmin = true only when that status indicates forbidden, falling back to text checks only if no status/code is present; ensure the api wrapper (and backend /api/admin/dead-letters) returns/throws an error object containing a status/code so the catch block can reliably use that property, and keep assigning error = msg for other cases..env.example (1)
133-154: 💤 Low valueConsider standardizing the environment variable prefix.
The AaaS configuration uses two different prefixes:
AWARENESS_*for most settings andAAAS_*for admin/JWT settings. This inconsistency may confuse developers during setup.Suggestion: Unify under AWARENESS_ prefix
-AAAS_ADMIN_ENAMES="" +AWARENESS_ADMIN_ENAMES="" -AAAS_JWT_SECRET="replace-with-a-strong-secret" +AWARENESS_JWT_SECRET="replace-with-a-strong-secret"🤖 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 @.env.example around lines 133 - 154, The .env.example mixes prefixes (AWARENESS_* vs AAAS_*); rename AAAS_ADMIN_ENAMES -> AWARENESS_ADMIN_ENAMES and AAAS_JWT_SECRET -> AWARENESS_JWT_SECRET in the .env.example and update any code that reads AAAS_ADMIN_ENAMES or AAAS_JWT_SECRET (search for symbols AAAS_ADMIN_ENAMES and AAAS_JWT_SECRET) to use the new AWARENESS_* names so the environment variable naming is consistent across config and code.services/awareness-service/api/package.json (1)
8-8: 💤 Low valueConsider removing redundant --exec flag.
The
nodemon.jsonconfig already specifies"exec": "ts-node src/index.ts"(line 5 in nodemon.json). The--execflag here may override it or cause confusion about which takes precedence.Suggested simplification
- "dev": "nodemon --exec ts-node src/index.ts", + "dev": "nodemon",The nodemon.json configuration will be used automatically.
🤖 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 `@services/awareness-service/api/package.json` at line 8, The dev script in package.json currently uses an explicit "--exec ts-node src/index.ts" which duplicates the "exec": "ts-node src/index.ts" already declared in nodemon.json; open the package.json entry for the "dev" script and remove the redundant --exec flag so the script simply runs "nodemon" (allowing nodemon.json's exec setting to take effect), keeping nodemon.json's "exec" value as the single source of truth.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 00ec4be5-c797-4c1f-b745-cbab17a493ed
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (58)
.env.exampledocs/docs/Services/Awareness-as-a-Service.mddocs/docs/Services/_category_.jsoninfrastructure/evault-core/src/core/protocol/graphql-server.tspnpm-workspace.yamlservices/awareness-service/README.mdservices/awareness-service/api/nodemon.jsonservices/awareness-service/api/package.jsonservices/awareness-service/api/src/config.tsservices/awareness-service/api/src/controllers/AdminController.tsservices/awareness-service/api/src/controllers/ApplicationController.tsservices/awareness-service/api/src/controllers/AuthController.tsservices/awareness-service/api/src/controllers/ConsumerController.tsservices/awareness-service/api/src/controllers/IngestController.tsservices/awareness-service/api/src/controllers/QueryController.tsservices/awareness-service/api/src/controllers/SubscriptionController.tsservices/awareness-service/api/src/database/data-source.tsservices/awareness-service/api/src/database/entities/AccessApplication.tsservices/awareness-service/api/src/database/entities/ApiKey.tsservices/awareness-service/api/src/database/entities/Consumer.tsservices/awareness-service/api/src/database/entities/DeadLetter.tsservices/awareness-service/api/src/database/entities/Delivery.tsservices/awareness-service/api/src/database/entities/Packet.tsservices/awareness-service/api/src/database/entities/Subscription.tsservices/awareness-service/api/src/database/migrations/1715200000000-Init.tsservices/awareness-service/api/src/index.tsservices/awareness-service/api/src/middleware/consumerAuth.tsservices/awareness-service/api/src/middleware/portalAuth.tsservices/awareness-service/api/src/openapi.tsservices/awareness-service/api/src/scripts/backfill-neo4j.tsservices/awareness-service/api/src/scripts/seed-catchall.tsservices/awareness-service/api/src/services/ApiKeyService.tsservices/awareness-service/api/src/services/DeliveryEngine.tsservices/awareness-service/api/src/services/IngestService.tsservices/awareness-service/api/src/services/SeedService.tsservices/awareness-service/api/src/services/SubscriptionMatcher.tsservices/awareness-service/api/src/services/W3dsAuthService.tsservices/awareness-service/api/src/types.tsservices/awareness-service/api/src/utils/backoff.tsservices/awareness-service/api/src/utils/cursor.tsservices/awareness-service/api/tsconfig.jsonservices/awareness-service/package.jsonservices/awareness-service/portal/package.jsonservices/awareness-service/portal/src/app.cssservices/awareness-service/portal/src/app.d.tsservices/awareness-service/portal/src/app.htmlservices/awareness-service/portal/src/lib/api.tsservices/awareness-service/portal/src/lib/ontology.tsservices/awareness-service/portal/src/lib/session.tsservices/awareness-service/portal/src/routes/+layout.svelteservices/awareness-service/portal/src/routes/+page.svelteservices/awareness-service/portal/src/routes/admin/+page.svelteservices/awareness-service/portal/src/routes/admin/dead-letters/+page.svelteservices/awareness-service/portal/src/routes/apply/+page.svelteservices/awareness-service/portal/src/routes/dashboard/+page.svelteservices/awareness-service/portal/svelte.config.jsservices/awareness-service/portal/tsconfig.jsonservices/awareness-service/portal/vite.config.ts
Comment on lines
+17
to
+30
| "dependencies": { | ||
| "@scalar/express-api-reference": "^0.9.16", | ||
| "axios": "^1.6.7", | ||
| "cors": "^2.8.5", | ||
| "dotenv": "^16.4.5", | ||
| "express": "^4.18.2", | ||
| "jsonwebtoken": "^9.0.2", | ||
| "neo4j-driver": "^5.28.1", | ||
| "pg": "^8.11.3", | ||
| "reflect-metadata": "^0.2.1", | ||
| "signature-validator": "workspace:*", | ||
| "typeorm": "^0.3.24", | ||
| "uuid": "^9.0.1" | ||
| }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for latest versions and security advisories for key dependencies
echo "=== Latest versions on npm ==="
for pkg in axios jsonwebtoken express pg typeorm; do
echo -n "$pkg: "
curl -s "https://registry.npmjs.org/$pkg/latest" | jq -r '.version'
done
echo -e "\n=== Security advisories check ==="
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "axios") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "jsonwebtoken") {
nodes {
advisory { summary severity publishedAt }
vulnerableVersionRange
firstPatchedVersion { identifier }
}
}
}'Repository: MetaState-Prototype-Project/prototype
Length of output: 4018
Update axios to patch critical security vulnerabilities.
Axios version 1.6.7 is vulnerable to multiple security issues, including a HIGH-severity Prototype Pollution vulnerability (CVE affecting versions >= 1.0.0, < 1.15.1). Additional vulnerabilities include SSRF via no_proxy bypass, CRLF injection, and DoS via unbounded recursion. Upgrade to at least 1.15.1 (latest is 1.16.1).
jsonwebtoken 9.0.2 is safe; no update needed.
🤖 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 `@services/awareness-service/api/package.json` around lines 17 - 30, The
package.json dependency for axios is pinned to "axios": "^1.6.7" which is
vulnerable; update the axios entry in
services/awareness-service/api/package.json to a fixed safe release (e.g.,
"^1.15.1" or the latest "^1.16.1"), then regenerate your lockfile by running
your package manager (npm/yarn/pnpm install) so package-lock.json or equivalent
is updated; finally run npm audit (or your CI security checks) and the test
suite to verify nothing breaks.
| databaseUrl: process.env.AWARENESS_DATABASE_URL ?? "", | ||
| apiPort: parseInt(process.env.AWARENESS_API_PORT ?? "4100", 10), | ||
| /** Shared secret evault-core must present on POST /ingest. */ | ||
| ingestSecret: process.env.AWARENESS_INGEST_SECRET ?? "", |
Contributor
There was a problem hiding this comment.
Shared ingest secret must be required.
The ingestSecret defaults to an empty string, which means the /ingest endpoint will accept packets from anyone. This is a critical authentication bypass.
🔒 Proposed fix
- ingestSecret: process.env.AWARENESS_INGEST_SECRET ?? "",
+ ingestSecret: required("AWARENESS_INGEST_SECRET"),🤖 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 `@services/awareness-service/api/src/config.ts` at line 19, The ingestSecret in
config.ts is defaulting to an empty string which allows unauthenticated ingests;
change the code that sets ingestSecret to require the AWARENESS_INGEST_SECRET
env var, validate that process.env.AWARENESS_INGEST_SECRET is present and
non-empty, and if not throw an Error (or exit early) during startup so the
service fails fast. Update the ingestSecret assignment (symbol: ingestSecret) to
use the validated value rather than a fallback empty string and add a clear
error message indicating the missing AWARENESS_INGEST_SECRET.
Comment on lines
+17
to
+27
| function normaliseTarget( | ||
| targetUrl: unknown, | ||
| webhookBaseUrl: string | null, | ||
| ): string | null { | ||
| if (typeof targetUrl === "string" && targetUrl.trim()) { | ||
| return targetUrl.trim(); | ||
| } | ||
| if (webhookBaseUrl) { | ||
| return new URL("/api/webhook", webhookBaseUrl).toString(); | ||
| } | ||
| return null; |
Contributor
There was a problem hiding this comment.
Validate subscription targets before saving them.
normaliseTarget() currently accepts any trimmed string, and the delivery worker will later POST to it. That makes this endpoint an SSRF primitive for any approved consumer, including metadata and private-network destinations.
🛡️ Minimum validation to add here
function normaliseTarget(
targetUrl: unknown,
webhookBaseUrl: string | null,
): string | null {
if (typeof targetUrl === "string" && targetUrl.trim()) {
- return targetUrl.trim();
+ try {
+ const url = new URL(targetUrl.trim());
+ if (!["http:", "https:"].includes(url.protocol)) return null;
+ return url.toString();
+ } catch {
+ return null;
+ }
}
if (webhookBaseUrl) {
- return new URL("/api/webhook", webhookBaseUrl).toString();
+ const url = new URL("/api/webhook", webhookBaseUrl);
+ if (!["http:", "https:"].includes(url.protocol)) return null;
+ return url.toString();
}
return null;
}You’ll still want host-level SSRF protection on top of this; protocol checks alone won’t block localhost, link-local, or RFC1918 destinations.
Also applies to: 43-67, 76-82
🤖 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 `@services/awareness-service/api/src/controllers/SubscriptionController.ts`
around lines 17 - 27, normaliseTarget currently accepts any trimmed string which
enables SSRF; update normaliseTarget to parse the value with the URL constructor
(or return the webhookBaseUrl fallback if provided), then enforce that protocol
is "http:" or "https:", reject if hostname is "localhost" or a literal IP in
RFC1918/loopback/link-local ranges (and optionally IPv6 equivalents), and return
null for invalid destinations; apply the same validation logic to the other
subscription target handling blocks referenced (the code around lines 43-67 and
76-82) so all entry points use the same URL parsing, protocol check, and
private/loopback IP rejection before saving.
|
|
||
| await queryRunner.query(` | ||
| CREATE TABLE "consumers" ( | ||
| "id" uuid NOT NULL DEFAULT gen_random_uuid(), |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether migrations use gen_random_uuid() without provisioning pgcrypto.
rg -n "gen_random_uuid\\(|CREATE EXTENSION\\s+IF NOT EXISTS\\s+\"?pgcrypto\"?" services/awareness-service/api/src/database/migrationsRepository: MetaState-Prototype-Project/prototype
Length of output: 936
Add CREATE EXTENSION IF NOT EXISTS "pgcrypto" before using gen_random_uuid().
The migration uses gen_random_uuid() in 6 DEFAULT clauses (lines 46, 63, 81, 100, 118, 141) but never provisions the required pgcrypto extension. Migration will fail immediately on fresh PostgreSQL instances.
Proposed fix
public async up(queryRunner: QueryRunner): Promise<void> {
+ await queryRunner.query(`CREATE EXTENSION IF NOT EXISTS "pgcrypto"`);
+
await queryRunner.query(`
CREATE TABLE "packets" (🤖 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 `@services/awareness-service/api/src/database/migrations/1715200000000-Init.ts`
at line 46, The migration uses gen_random_uuid() in several DEFAULT clauses but
never provisions the pgcrypto extension; update the migration
(1715200000000-Init.ts) to run CREATE EXTENSION IF NOT EXISTS "pgcrypto" at the
start of the migration (e.g. in the up migration before any CREATE TABLE
statements) so gen_random_uuid() is available for all DEFAULT uuid expressions;
ensure the extension creation is idempotent and placed before the first use of
gen_random_uuid().
Comment on lines
+151
to
+174
| if (attempts >= config.maxAttempts) { | ||
| await deliveryRepo.update(delivery.id, { | ||
| status: "failed", | ||
| attempts, | ||
| lastError: message, | ||
| lastResponseStatus: responseStatus, | ||
| }); | ||
| await AppDataSource.getRepository(DeadLetter).insert({ | ||
| deliveryId: delivery.id, | ||
| subscriptionId: delivery.subscriptionId, | ||
| packetId: delivery.packetId, | ||
| consumerId: subscription?.consumerId ?? delivery.subscriptionId, | ||
| payload: (ctx?.payload ?? {}) as any, | ||
| targetUrl: subscription?.targetUrl ?? "", | ||
| totalAttempts: attempts, | ||
| lastError: message, | ||
| lastResponseStatus: responseStatus, | ||
| resolved: false, | ||
| }); | ||
| console.warn( | ||
| `[aaas] delivery ${delivery.id} dead-lettered after ${attempts} attempts`, | ||
| ); | ||
| return; | ||
| } |
Contributor
There was a problem hiding this comment.
Terminal failures are re-queued indefinitely after dead-lettering.
Line 153 keeps status as "failed" and nextAttemptAt is not advanced in this branch, but Line 65 + Line 66 re-claim failed rows. This can repeatedly resend and repeatedly dead-letter the same delivery.
Suggested change
if (attempts >= config.maxAttempts) {
await deliveryRepo.update(delivery.id, {
- status: "failed",
+ status: "dead_lettered",
attempts,
lastError: message,
lastResponseStatus: responseStatus,
+ nextAttemptAt: null as any,
});- WHERE status IN ('pending', 'failed')
+ WHERE status IN ('pending', 'failed')
AND "nextAttemptAt" <= now()If introducing dead_lettered is not possible right now, at least set nextAttemptAt far in the future in terminal branch so it is not re-claimed.
🤖 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 `@services/awareness-service/api/src/services/DeliveryEngine.ts` around lines
151 - 174, The terminal branch that marks deliveries as failed (inside the
attempts >= config.maxAttempts block) leaves status as "failed" and does not
advance nextAttemptAt, allowing re-claiming by the handler that considers
"failed" rows; update the call to deliveryRepo.update for delivery.id to either
set status to a distinct terminal value like "dead_lettered" (preferred) or, if
that status can't be introduced now, set nextAttemptAt to a far-future timestamp
(e.g., now + many years) so the row is not re-claimed; ensure the DeadLetter
insert remains unchanged and reference deliveryRepo.update, delivery.id,
attempts, and config.maxAttempts when making the change.
Comment on lines
+24
to
+26
| "dependencies": { | ||
| "svelte-qrcode": "^1.0.1" | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check svelte-qrcode package metadata and recent issues for Svelte 5 compatibility
echo "=== NPM Package Info ==="
curl -s https://registry.npmjs.org/svelte-qrcode/1.0.1 | jq '{version: .version, peerDependencies: .peerDependencies, dependencies: .dependencies}'
echo -e "\n=== GitHub Issues Check ==="
gh api graphql -f query='
{
search(query: "repo:JonasKruckenberg/svelte-qrcode svelte 5 OR runes in:issue,comments", type: ISSUE, first: 5) {
nodes {
... on Issue {
title
state
url
createdAt
}
}
}
}'Repository: MetaState-Prototype-Project/prototype
Length of output: 268
🏁 Script executed:
#!/bin/bash
# Check package maintenance status and last update
echo "=== Package Metadata (Full) ==="
curl -s https://registry.npmjs.org/svelte-qrcode/1.0.1 | jq '{name: .name, version: .version, time: .time, keywords: .keywords, repository: .repository, author: .author, maintainers: .maintainers}'
echo -e "\n=== Latest version available ==="
curl -s https://registry.npmjs.org/svelte-qrcode | jq '.["dist-tags"]'
echo -e "\n=== Package.json from latest ==="
curl -s https://registry.npmjs.org/svelte-qrcode | jq '.versions | keys[-1] as $latest | .[$latest] | {version, engines, svelte}'Repository: MetaState-Prototype-Project/prototype
Length of output: 626
🌐 Web query:
svelte-qrcode Svelte 5 compatibility
💡 Result:
The original svelte-qrcode package is not compatible with Svelte 5 [1][2]. Because Svelte 5 introduced significant changes to reactivity—specifically the move to runes like $state, $derived, and @kodav.dev/svelte5-qrcode, which is a fork of the original library explicitly maintained for Svelte 5 compatibility [4][5]. Summary of options: - Avoid the original svelte-qrcode package, as it is unmaintained and lacks Svelte 5 support [1][2]. - Use @kodav.dev/svelte5-qrcode for native Svelte 5 support, including full runes integration and TypeScript type safety [4][5]. - Alternatively, you can use a general-purpose QR code library (such as the npm qrcode package) and wrap it in a custom Svelte 5 component using $effect to handle the rendering logic [6].
Citations:
- 1: https://registry.npmjs.org/@trasherdk/svelte-qrcode
- 2: https://github.com/trasherdk/svelte-qrcode
- 3: https://dev.to/digital-abetka/svelte-5-ecosystem-whats-new-why-switch-33m
- 4: https://github.com/kodav-dev/svelte5-qrcode
- 5: https://www.npmjs.com/package/@kodav.dev/svelte5-qrcode
- 6: Does this not work for sveltekit? JonasJs/svelte-qrcode#8
Replace svelte-qrcode with a Svelte 5-compatible alternative.
svelte-qrcode@1.0.1 is incompatible with Svelte 5. The package uses Svelte 3/4 syntax (export let, $: reactive statements) and is unmaintained, making it non-functional in Svelte 5 projects that require runes ($state, $derived, $props).
Replace with @kodav.dev/svelte5-qrcode (a Svelte 5 fork with full runes support) or use a generic QR code library (e.g., qrcode) and wrap it in a custom Svelte 5 component.
🤖 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 `@services/awareness-service/portal/package.json` around lines 24 - 26, Replace
the incompatible "svelte-qrcode" dependency with a Svelte 5-compatible option by
updating the dependency entry in package.json: remove "svelte-qrcode": "^1.0.1"
and add either "`@kodav.dev/svelte5-qrcode`": "<latest>" or a generic "qrcode":
"<latest>" (if using a wrapper component). Then update any imports/usages that
reference "svelte-qrcode" in your Svelte components to import the new package or
your custom QR wrapper component (search for import "svelte-qrcode" and
component tags using it), and run the package manager (npm/yarn/pnpm) to
reinstall and update lockfiles.
- Add a full OpenAPI 3.1 document covering every AaaS endpoint, served raw at GET /openapi.json and rendered as an interactive Scalar reference at GET /docs. - Add a Services section to the docs site with an Awareness as a Service page covering architecture, packet format, capabilities, auth, migration and configuration. - Note the docs endpoints in the service README.
The Neo4j backfill now reads the root .env's NEO4J_URI / NEO4J_USER / NEO4J_PASSWORD - the same connection evault-core uses - instead of duplicate AWARENESS_NEO4J_* vars. Drop the AaaS-specific Neo4j vars from .env.example and the docs.
DataSource.query() with UPDATE ... RETURNING returned a [rows, affectedCount]
tuple, so the delivery engine iterated the tuple instead of the rows - each
'delivery' had no id and Repository.update threw 'Empty criteria(s) are not
allowed'. Claim the batch with the query builder's update().returning('*')
instead, whose .raw is a well-defined rows array, and skip any row missing an
id defensively.
Use $env/static/public so PUBLIC_AWARENESS_API_URL is baked into the portal bundle at build time, instead of $env/dynamic/public which reads process.env at runtime and fell back to localhost under pm2.
- Subscription form now selects ontologies from the ontology service (https://ontology.w3ds.metastate.foundation/schemas) instead of free text, and takes eVault filters as a tag input. - Dashboard links straight to the interactive API reference, built from the API base URL. - The session JWT now carries an isAdmin claim; the portal hides the Admin nav link from non-admins.
- An access application now collects only platform name, website URL and description; contact email and requested-ontologies fields are dropped. - Restyle the entire portal (layout, login, apply, dashboard, admin, dead-letters) with a dark theme.
The OpenAPI document / Scalar reference now only describes endpoints a consuming platform integrates against - packet polling, webhook subscriptions and consumer self-service. The ingest endpoint, W3DS portal auth, access applications and admin routes are no longer exposed in the docs.
Fix the broken express setup and make it a tiny inlet that logs the method, path, headers and body of any request on any path, and replies 200 - handy for eyeballing AaaS webhook deliveries.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts`:
- Line 19: The test suite mutates process.env.AWARENESS_SERVICE_URL directly;
capture the original value before changing it and restore it in an afterAll (or
afterEach) teardown to avoid leaking env changes to other tests—wrap the
assignment (process.env.AWARENESS_SERVICE_URL = "http://localhost:9999") with a
saved const original = process.env.AWARENESS_SERVICE_URL and ensure the teardown
resets process.env.AWARENESS_SERVICE_URL = original (apply the same pattern for
the other occurrences around lines 34-39 in graphql-server.spec.ts).
- Around line 241-254: The test currently grabs the first ingest call via
ingestCalls[0], which is fragile; instead, find the call whose payload.id
matches the expected envelopeId. Update the logic that builds ingestCalls (the
axios.post mock filtering) to locate the correct call with ingestCalls.find(call
=> call[1]?.id === envelopeId) and set ingestPayload to that result (asserting
it exists) so the subsequent expectation on ingestPayload.id is deterministic;
refer to axios.post mock, the ingestCalls variable, and the envelopeId used
later in the test.
In `@services/awareness-service/api/src/controllers/AdminController.ts`:
- Around line 43-53: The approve/reject and dead-letter replay flows currently
update AccessApplication (variable application via appRepo.save) and Consumer
(via AppDataSource.getRepository(Consumer).update) separately which can leave
inconsistent state if one write fails; wrap those paired updates in a single
AppDataSource.transaction(...) and perform both writes inside the transaction
(set application.status/reviewedAt/reviewedByEname/reviewNote on the application
object, then use the transaction's manager to save the application and to update
the Consumer repository in the same transaction) so both changes commit or both
roll back; apply the same transactional pattern to the reject endpoint and the
dead-letter replay block referenced around lines 111–122.
- Around line 111-123: The Delivery.update call in the AdminController /replay
flow must be checked for its result before marking the deadLetter resolved;
capture the result of
AppDataSource.getRepository(Delivery).update(deadLetter.deliveryId, ...) and
verify result.affected > 0, and only then set deadLetter.resolved = true and
call dlRepo.save(deadLetter) and return success; if affected is 0 return an
appropriate error/404 response instead so the dead letter isn’t prematurely
cleared.
In `@services/awareness-service/api/src/openapi.ts`:
- Around line 52-58: The OpenAPI 3.1 schema uses OpenAPI 3.0's nullable: true;
update all occurrences (e.g., evaultPublicKey, w3id, data and other fields
referenced) to JSON Schema null unions by replacing nullable: true with type as
a union including "null" (for example change type: "string", nullable: true →
type: ["string","null"]; for objects use type: ["object","null"] and keep
additionalProperties as needed). Ensure every field listed in the comment (lines
referencing evaultPublicKey, w3id, data and the other affected entries) uses the
union-style nullability so the document is valid under OpenAPI 3.1.
🪄 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: 5df851b6-aef3-40ac-940a-5bbb92f5f671
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (11)
infrastructure/evault-core/src/core/protocol/graphql-server.spec.tsservices/awareness-service/api/package.jsonservices/awareness-service/api/src/controllers/AdminController.tsservices/awareness-service/api/src/controllers/QueryController.tsservices/awareness-service/api/src/openapi.tsservices/awareness-service/api/src/scripts/backfill-neo4j.tsservices/awareness-service/api/src/services/DeliveryEngine.tsservices/awareness-service/portal/src/lib/types/svelte-qrcode.d.tsservices/awareness-service/portal/src/routes/+page.svelteservices/webhook-inlet-test/index.jsservices/webhook-inlet-test/package.json
✅ Files skipped from review due to trivial changes (3)
- services/awareness-service/portal/src/lib/types/svelte-qrcode.d.ts
- services/webhook-inlet-test/index.js
- services/webhook-inlet-test/package.json
🚧 Files skipped from review as they are similar to previous changes (4)
- services/awareness-service/api/package.json
- services/awareness-service/portal/src/routes/+page.svelte
- services/awareness-service/api/src/services/DeliveryEngine.ts
- services/awareness-service/api/src/scripts/backfill-neo4j.ts
| describe("GraphQLServer Webhook Payload W3ID", () => { | ||
| // evault-core forwards every awareness packet to AaaS at | ||
| // AWARENESS_SERVICE_URL/ingest; point it somewhere the spy can intercept. | ||
| process.env.AWARENESS_SERVICE_URL = "http://localhost:9999"; |
Contributor
There was a problem hiding this comment.
Restore AWARENESS_SERVICE_URL after the suite.
The suite mutates process environment globally but does not restore it, which can leak behavior into other tests.
Suggested fix
+const previousAwarenessServiceUrl = process.env.AWARENESS_SERVICE_URL;
process.env.AWARENESS_SERVICE_URL = "http://localhost:9999";
...
afterAll(async () => {
await teardownE2ETestServer(server);
+ process.env.AWARENESS_SERVICE_URL = previousAwarenessServiceUrl;
if (axiosPostSpy) {
axiosPostSpy.mockRestore();
}
});Also applies to: 34-39
🤖 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 `@infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts` at line
19, The test suite mutates process.env.AWARENESS_SERVICE_URL directly; capture
the original value before changing it and restore it in an afterAll (or
afterEach) teardown to avoid leaking env changes to other tests—wrap the
assignment (process.env.AWARENESS_SERVICE_URL = "http://localhost:9999") with a
saved const original = process.env.AWARENESS_SERVICE_URL and ensure the teardown
resets process.env.AWARENESS_SERVICE_URL = original (apply the same pattern for
the other occurrences around lines 34-39 in graphql-server.spec.ts).
Comment on lines
+241
to
254
| // Give the fire-and-forget ingest call a moment to run. | ||
| await new Promise(resolve => setTimeout(resolve, 1000)); | ||
|
|
||
| // Verify axios.post was called (webhook delivery) | ||
| // Verify axios.post was called (awareness ingest) | ||
| expect(axios.post).toHaveBeenCalled(); | ||
| // Get the webhook payload | ||
| const webhookCalls = (axios.post as any).mock.calls.filter((call: any[]) => | ||
| typeof call[0] === "string" && call[0].includes("/api/webhook") | ||
|
|
||
| // Get the ingest payload | ||
| const ingestCalls = (axios.post as any).mock.calls.filter((call: any[]) => | ||
| typeof call[0] === "string" && call[0].includes("/ingest") | ||
| ); | ||
|
|
||
| expect(webhookCalls.length).toBeGreaterThan(0); | ||
| const webhookPayload = webhookCalls[0][1]; | ||
| expect(ingestCalls.length).toBeGreaterThan(0); | ||
| const ingestPayload = ingestCalls[0][1]; | ||
|
|
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts | sed -n '200,280p'Repository: MetaState-Prototype-Project/prototype
Length of output: 3029
🏁 Script executed:
cat -n infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts | sed -n '170,210p'Repository: MetaState-Prototype-Project/prototype
Length of output: 1883
🏁 Script executed:
rg -n "ingestCalls\[0\]" infrastructure/evault-core/src/core/protocol/graphql-server.spec.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 138
🏁 Script executed:
rg -n "mock.calls.filter" infrastructure/evault-core/src/core/protocol/graphql-server.spec.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 263
🏁 Script executed:
cat -n infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts | sed -n '140,175p'Repository: MetaState-Prototype-Project/prototype
Length of output: 1650
Replace blind index access with deterministic ingest matching by ID.
The code uses ingestCalls[0] to select the ingest payload, but this is fragile when multiple ingest calls may be present. The test already validates ingestPayload.id against envelopeId (line 259), so use .find() to deterministically match by ID instead of assuming the first call is correct. This matches the robust pattern already used in the createMetaEnvelope test above (lines 162-167).
🤖 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 `@infrastructure/evault-core/src/core/protocol/graphql-server.spec.ts` around
lines 241 - 254, The test currently grabs the first ingest call via
ingestCalls[0], which is fragile; instead, find the call whose payload.id
matches the expected envelopeId. Update the logic that builds ingestCalls (the
axios.post mock filtering) to locate the correct call with ingestCalls.find(call
=> call[1]?.id === envelopeId) and set ingestPayload to that result (asserting
it exists) so the subsequent expectation on ingestPayload.id is deterministic;
refer to axios.post mock, the ingestCalls variable, and the envelopeId used
later in the test.
Comment on lines
+43
to
+53
| application.status = "approved"; | ||
| application.reviewedByEname = req.ename!; | ||
| application.reviewNote = req.body?.note ?? null; | ||
| application.reviewedAt = new Date(); | ||
| await appRepo.save(application); | ||
|
|
||
| await AppDataSource.getRepository(Consumer).update( | ||
| application.consumerId, | ||
| { status: "approved", approvedAt: new Date() }, | ||
| ); | ||
| res.json({ ok: true, application }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "AdminController.ts" | head -20Repository: MetaState-Prototype-Project/prototype
Length of output: 207
🏁 Script executed:
wc -l ./services/awareness-service/api/src/controllers/AdminController.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 153
🏁 Script executed:
cat -n ./services/awareness-service/api/src/controllers/AdminController.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 5654
🏁 Script executed:
cat ./services/awareness-service/api/src/database/data-source.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1200
🏁 Script executed:
rg "transaction\|queryRunner\|AppDataSource\.transaction" --type tsRepository: MetaState-Prototype-Project/prototype
Length of output: 63
🏁 Script executed:
rg "AppDataSource" --type ts -A 2 | head -80Repository: MetaState-Prototype-Project/prototype
Length of output: 7522
🏁 Script executed:
cat ./services/awareness-service/api/src/database/entities/Consumer.ts ./services/awareness-service/api/src/database/entities/AccessApplication.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 2402
Wrap approve/reject state transitions in a transaction.
The approve and reject endpoints write AccessApplication and Consumer separately. If the second write fails, states diverge (e.g., approved application with pending consumer status). Use AppDataSource.transaction() to atomically update both entities.
Also applies to the dead-letter replay endpoint (lines 111–122).
🤖 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 `@services/awareness-service/api/src/controllers/AdminController.ts` around
lines 43 - 53, The approve/reject and dead-letter replay flows currently update
AccessApplication (variable application via appRepo.save) and Consumer (via
AppDataSource.getRepository(Consumer).update) separately which can leave
inconsistent state if one write fails; wrap those paired updates in a single
AppDataSource.transaction(...) and perform both writes inside the transaction
(set application.status/reviewedAt/reviewedByEname/reviewNote on the application
object, then use the transaction's manager to save the application and to update
the Consumer repository in the same transaction) so both changes commit or both
roll back; apply the same transactional pattern to the reject endpoint and the
dead-letter replay block referenced around lines 111–122.
Comment on lines
+111
to
+123
| await AppDataSource.getRepository(Delivery).update( | ||
| deadLetter.deliveryId, | ||
| { | ||
| status: "pending", | ||
| attempts: 0, | ||
| nextAttemptAt: new Date(), | ||
| lastError: null, | ||
| lastResponseStatus: null, | ||
| }, | ||
| ); | ||
| deadLetter.resolved = true; | ||
| await dlRepo.save(deadLetter); | ||
| res.json({ ok: true }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the AdminController.ts file
find . -type f -name "AdminController.ts" | head -5Repository: MetaState-Prototype-Project/prototype
Length of output: 207
🏁 Script executed:
# Read the AdminController.ts file around lines 111-123 to see the full context
wc -l ./services/awareness-service/api/src/controllers/AdminController.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 153
🏁 Script executed:
# Read the entire AdminController.ts file to understand the full context
cat -n ./services/awareness-service/api/src/controllers/AdminController.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 5654
🏁 Script executed:
# Let's check the TypeORM version and understand the update() return behavior
# First, look for package.json to see the dependencies
find ./services/awareness-service -name "package.json" -type fRepository: MetaState-Prototype-Project/prototype
Length of output: 218
🏁 Script executed:
# Check the API package.json to see TypeORM version
cat ./services/awareness-service/api/package.jsonRepository: MetaState-Prototype-Project/prototype
Length of output: 1590
🏁 Script executed:
# Check the DeadLetter and Delivery entity definitions to understand the relationship
find ./services/awareness-service/api/src/database/entities -name "*.ts" -type fRepository: MetaState-Prototype-Project/prototype
Length of output: 561
🏁 Script executed:
# Read the Delivery entity
cat -n ./services/awareness-service/api/src/database/entities/Delivery.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1804
🏁 Script executed:
# Read the DeadLetter entity
cat -n ./services/awareness-service/api/src/database/entities/DeadLetter.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1605
🏁 Script executed:
# Let's verify TypeORM's update() return behavior by checking documentation or code
# First, let's search for any usage of update() result in the codebase
rg "\.update\(" ./services/awareness-service/api/src --context 3 -A 2Repository: MetaState-Prototype-Project/prototype
Length of output: 3925
🏁 Script executed:
# Let's see where the result of update() is actually being used
rg "const result = await.*\.update\(" ./services/awareness-service/api/src -A 5Repository: MetaState-Prototype-Project/prototype
Length of output: 1356
Check that the Delivery row was actually updated before resolving the dead letter.
The /replay endpoint calls Delivery.update() without verifying the result. If the deliveryId doesn't exist or the row is unmodified, affected will be 0, but the code unconditionally marks the dead letter as resolved and returns success. This loses retry visibility. Use the same pattern as SubscriptionController and ApiKeyService—check result.affected before proceeding.
🤖 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 `@services/awareness-service/api/src/controllers/AdminController.ts` around
lines 111 - 123, The Delivery.update call in the AdminController /replay flow
must be checked for its result before marking the deadLetter resolved; capture
the result of
AppDataSource.getRepository(Delivery).update(deadLetter.deliveryId, ...) and
verify result.affected > 0, and only then set deadLetter.resolved = true and
call dlRepo.save(deadLetter) and return success; if affected is 0 return an
appropriate error/404 response instead so the dead letter isn’t prematurely
cleared.
Comment on lines
+52
to
+58
| evaultPublicKey: { type: "string", nullable: true }, | ||
| w3id: { | ||
| type: "string", | ||
| nullable: true, | ||
| description: "Owner's W3ID (eName)", | ||
| }, | ||
| data: { type: "object", nullable: true, additionalProperties: true }, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd services/awareness-service/api/src && wc -l openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 96
🏁 Script executed:
cd services/awareness-service/api/src && head -20 openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 973
🏁 Script executed:
cd services/awareness-service/api/src && sed -n '1,60p' openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 2776
🏁 Script executed:
cd services/awareness-service/api/src && sed -n '75,90p' openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 731
🏁 Script executed:
cd services/awareness-service/api/src && sed -n '110,135p' openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 1163
🏁 Script executed:
cd services/awareness-service/api/src && sed -n '225,235p' openapi.tsRepository: MetaState-Prototype-Project/prototype
Length of output: 660
Use OpenAPI 3.1-compliant nullability syntax.
This document is declared as OpenAPI 3.1, but multiple fields use nullable: true, which is the OpenAPI 3.0 syntax. OpenAPI 3.1 adopted JSON Schema directly and requires nullability to be expressed with union types (for example type: ["string", "null"]). This will cause strict OpenAPI 3.1 validators and code generators to fail.
Suggested pattern
-evaultPublicKey: { type: "string", nullable: true },
+evaultPublicKey: { type: ["string", "null"] },
-nextCursor: { type: "string", nullable: true },
+nextCursor: { type: ["string", "null"] },Applies to: lines 52-58, 81-82, 114-116, 124-129, 231.
🤖 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 `@services/awareness-service/api/src/openapi.ts` around lines 52 - 58, The
OpenAPI 3.1 schema uses OpenAPI 3.0's nullable: true; update all occurrences
(e.g., evaultPublicKey, w3id, data and other fields referenced) to JSON Schema
null unions by replacing nullable: true with type as a union including "null"
(for example change type: "string", nullable: true → type: ["string","null"];
for objects use type: ["object","null"] and keep additionalProperties as
needed). Ensure every field listed in the comment (lines referencing
evaultPublicKey, w3id, data and the other affected entries) uses the union-style
nullability so the document is valid under OpenAPI 3.1.
The graphql-server spec asserted evault-core POSTed webhooks to platform /api/webhook endpoints. evault-core now forwards a single packet to AWARENESS_SERVICE_URL/ingest, so the spec intercepts /ingest instead, drops the obsolete /platforms mock, and no longer waits on the removed setTimeout fanout delay.
svelte-qrcode ships no types, so svelte-check (run by pnpm check in CI) failed on the portal with 'Cannot find module svelte-qrcode'. Add a module declaration shim, mirroring the one in platforms/enotary.
The Neo4j graph can hold multiple MetaEnvelope nodes with the same id, which made a batch upsert touch the same ON CONFLICT target twice - Postgres rejects that with 'cannot affect row a second time'. Collapse duplicate ids within each batch before upserting.
GET /api/packets now also returns count (packets in this page), total (all packets matching the filter), pageSize and totalPages alongside the existing cursor pagination. The total is computed with a filter-only COUNT, independent of the cursor.
…ry engine When Postgres restarts or is in recovery (57P03 and friends), the delivery tick threw every 2s and dumped a full stack trace each time. Detect transient DB-unavailable errors and log a single concise warning per outage; the engine resumes automatically once the database is back.
- GET /api/admin/dead-letters returns metadata only; the full webhook payload column is omitted so the list stays small. - Center the QR code within its white plate on the login screen.
The declared range was ^1.6.7 - a stale floor that permits axios versions with known CVEs. The lockfile already resolved to 1.13.6 (past the relevant fixes); raise the declared floor to ^1.13.6 so the range itself can no longer resolve to a vulnerable version.
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
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
New Features
Configuration
Documentation