[VW-71] Remediation and Device Artifact Integrations#76
Open
trummelhadron wants to merge 7 commits intomainfrom
Open
[VW-71] Remediation and Device Artifact Integrations#76trummelhadron wants to merge 7 commits intomainfrom
trummelhadron wants to merge 7 commits intomainfrom
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
added 2 commits
February 19, 2026 14:43
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds new external mapping models and migration; expands Prisma models (DeviceArtifact, Vulnerability, Remediation, Integration, ExternalAssetMapping); implements integrationUpload endpoints for DeviceArtifact and Remediation with sync logic changes (transactional create + artifact-wrapper creation); and adds integration-focused tests and test utilities. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Backend
participant Database
participant IntegrationAPI as External Integration
Client->>Backend: POST /deviceArtifacts/integrationUpload\n(API key, payload)
Backend->>Backend: Extract userId from context
Backend->>Database: prisma.integration.findFirst(apiKey)
Database-->>Backend: Integration record (or null)
Backend->>Backend: integrationId = requireExistence(integration).id
Backend->>Backend: For each item: transformInputItem -> {createData, updateData, artifactWrapperParentId, ...}
Backend->>Database: BEGIN TRANSACTION
Backend->>Database: Create or update item (deviceArtifact/remediation)
Database-->>Backend: Item created/updated
Backend->>Database: Create external mapping (external_*_mappings)
Database-->>Backend: Mapping created
Backend->>Database: Conditionally create ArtifactWrappers (use artifactWrapperParentId)
Database-->>Backend: Artifact wrappers created
Backend->>Database: COMMIT
Database-->>Backend: Transaction committed
Backend-->>Client: Return integrationResponse (counts, shouldRetry, message)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/v1/__tests__/deviceArtifacts.test.ts`:
- Around line 38-49: Rename the copy-paste name in the test payload: update the
mockIntegrationPayload.name (currently "mockVulnIntegration") to a clearer value
such as "mockDeviceArtifactIntegration" to match ResourceType.DeviceArtifact and
avoid confusion when reviewing tests that reference mockIntegrationPayload.
- Around line 128-135: The test "GET /deviceArtifacts/integrationUpload -
Without auth, should be 401" is using request(...).get(...) against the
POST-only endpoint `/deviceArtifacts/integrationUpload`; change the HTTP verb to
`.post()` so the test exercises the actual integrationUpload handler and asserts
401 and UNAUTHORIZED correctly (update the test statement and any request
invocation in this spec; look for the test name string and the
`/deviceArtifacts/integrationUpload` call to modify).
- Line 552: Remove the debug console output by deleting the
console.log(integrationRes) statement in the test (the integrationRes variable
is the unique symbol to look for in the deviceArtifacts test). If you need to
retain visibility for local debugging, replace it with a conditional or use a
test logger at debug level (not console.log) so CI stays quiet. Ensure no other
stray console.log calls remain in the same test file.
In `@src/app/api/v1/__tests__/remediations.test.ts`:
- Around line 139-144: The test "GET /remediations/integrationUpload - Without
auth, should be 401" is calling request(...).get() but the application registers
the integrationUpload route as POST; change the test to call
request(BASE_URL).post(`/remediations/integrationUpload`) (include an empty body
or {} as the request payload) so it actually exercises the POST handler's auth
middleware; keep the same assertions on status and res.body.code.
- Line 589: The test uses an unsupported matcher toBeNullable() on
foundSync.errorMessage which causes runtime failures; replace toBeNullable()
with the built-in matcher toBeNull() (e.g., change
expect(foundSync.errorMessage).toBeNullable() to
expect(foundSync.errorMessage).toBeNull()) across the tests
(remediations.test.ts, deviceArtifacts.test.ts, assets.test.ts,
vulnerabilities.test.ts), or alternatively install and configure jest-extended
or vitest-jest-extended in your test setup (vitest.setup.ts) so toBeNullable()
becomes available; update assertions or test setup accordingly and run tests to
confirm.
In `@src/features/device-artifacts/server/routers.ts`:
- Around line 254-258: In updateData where you currently have deviceGroup: {
set: newDeviceGroup.id }, change it to the to-one relation pattern used
elsewhere: deviceGroup: { connect: { id: newDeviceGroup.id } } so Prisma
performs a proper many-to-one connect; update the updateData object (matching
the existing createData/deviceGroup usage and the pattern at the other
occurrence) to use connect instead of set.
In `@src/features/integrations/types.ts`:
- Around line 34-41: The remediations entry in integrationsMapping uses a plural
display name causing inconsistent UI labels; update the remediations.name value
from "Remediations" to the singular "Remediation" so it matches other entries
and the ResourceType.Remediation enum, ensuring the UI render of {value.name}
Integrations produces "Remediation Integrations"; locate the integrationsMapping
object and change the remediations.name property accordingly (the key is
remediations and the mapping is referenced by integrations-layout.tsx when
rendering {value.name}).
In `@src/lib/router-utils.ts`:
- Around line 153-155: Remove the debug comment line and the commented-out
fallback, and fix the error handling around the unknown variable `e` in the
function that currently returns `e.message`; narrow the type before accessing
`.message` (e.g., check `e instanceof Error` and return `e.message` only in that
case) and otherwise return a safe generic string like "Internal Server Error" so
we don't leak internals or cause a TypeScript error.
- Around line 315-338: processIntegrationSync currently calls
createArtifactWrappers(tx, item.artifacts, createdItem.id, "remediationId",
userId) with a hardcoded "remediationId" which causes FK mismatches for
device-artifacts; change the design so artifact parent field is configurable:
add optional properties to SyncConfig (e.g., parentField or artifactParentField)
or move artifact creation into each router's transformInputItem, then replace
the hardcoded string in processIntegrationSync with the config-provided field
(or call the router-specific artifact creator); also update the transaction
callback signature to type tx explicitly and narrow TInputItem to include an
optional artifacts property (or use a guarded type) so accessing item.artifacts
is type-safe (refer to processIntegrationSync, createArtifactWrappers,
SyncConfig, transformInputItem, and TInputItem).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/app/api/v1/__tests__/deviceArtifacts.test.ts (1)
535-543: BroaddeleteManycleanup could interfere with concurrent test runs.The
onTestFinishedcleanup deletes alldeviceArtifactrecords whose description contains"mock"(case-insensitive). If other tests (or parallel runs) create device artifacts with "mock" in the description, this cleanup will delete them too.Consider scoping cleanup to the specific records created, e.g., by collecting the created IDs from the integration response or from mapping queries, similar to how other tests in this file clean up by specific ID.
♻️ Proposed approach
+ // Collect created IDs for targeted cleanup + const createdMappings = await prisma.externalDeviceArtifactMapping.findMany({ + where: { integrationId: createdIntegration.id }, + select: { itemId: true }, + }); + const createdIds = createdMappings.map(m => m.itemId); + onTestFinished(async () => { - await prisma.deviceArtifact.deleteMany({ - where: { - description: { - contains: "mock", - mode: "insensitive" as const, - }, - }, + await prisma.deviceArtifact.deleteMany({ + where: { id: { in: createdIds } }, }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/v1/__tests__/deviceArtifacts.test.ts` around lines 535 - 543, The cleanup in onTestFinished currently calls prisma.deviceArtifact.deleteMany({... description: { contains: "mock", mode: "insensitive" }}) which is too broad and may remove artifacts from other tests; change the tests that create device artifacts to record the created artifact IDs (e.g., capture IDs from the integration response or a mapping query in the same test) and update onTestFinished to delete only those specific IDs using prisma.deviceArtifact.deleteMany({ where: { id: { in: recordedIds } } }) or similar; ensure the recordedIds collection is accessible to the teardown (declare it in the test file scope and push IDs from functions that create artifacts) and fall back to a safe no-op if the list is empty.src/features/device-artifacts/server/routers.ts (1)
220-228: Refactor the tRPC context type to properly type theauthproperty added byprotectedProcedure.The
@ts-expect-erroron line 223 works around a deeper issue: theContexttype defined insrc/trpc/init.tsonly includes{ req }, butprotectedProceduredynamically adds anauthproperty with a conditional structure (session flow setskey: undefined; API key flow setskeywith userId). This pattern repeats across at least four routers (vulnerabilities, remediations, device-artifacts, assets), all using the same suppression.Instead of suppressing the error at each call site, extend the
Contexttype to account for both auth flows, or create aProtectedContexttype that explicitly types the auth property with its conditional structure. This aligns with strict TypeScript mode expectations per the coding guidelines.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/features/device-artifacts/server/routers.ts` around lines 220 - 228, The ts-expect-error on ctx.auth.key arises because the Context type in src/trpc/init.ts only includes { req } while protectedProcedure injects an auth property with two flows; update the Context typing (or add a new ProtectedContext) to explicitly model auth as a discriminated/union type (e.g., session flow with key: undefined and api-key flow with key containing id and userId) so usages like ctx.auth and ctx.auth.key?.id in routers (e.g., device-artifacts router where prisma.integration.findFirst and requireExistence are used) are correctly typed; after adding the new type, switch the router signatures to use the updated Context/ProtectedContext and remove the `@ts-expect-error` suppression.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/lib/router-utils.ts`:
- Around line 315-328: The create call inside prisma.$transaction is using the
standalone config.model.create (config.model.create) so it runs outside the
transaction and breaks atomicity; update SyncConfig to expose a way to resolve
the model delegate from a transaction client (e.g., a method like
getModelFromTx(tx) or withTxModel(tx)) and in processIntegrationSync replace
config.model.create with the tx-bound delegate (use the delegate returned by
your new SyncConfig method) so that the created item and subsequent
createArtifactWrappers(tx, ...) calls all run on the same tx; apply the same
change to the other three callers (assets, vulnerabilities, remediations,
device-artifacts) so every model operation inside prisma.$transaction uses the
tx-scoped delegate.
---
Duplicate comments:
In `@src/app/api/v1/__tests__/deviceArtifacts.test.ts`:
- Line 552: Remove the stray console.log(integrationRes) left in the test;
locate the expression in the deviceArtifacts.test.ts test case where
integrationRes is logged and delete that console.log call (or replace it with an
assertion that verifies integrationRes contents if the output was intended for
verification) so CI output is not noisy.
- Around line 128-135: The test "GET /deviceArtifacts/integrationUpload -
Without auth, should be 401" is using request(BASE_URL).get(...) against the
POST-only endpoint `/deviceArtifacts/integrationUpload`; update the test to use
request(BASE_URL).post(...) so it exercises the actual handler (no auth) and
still assert res.status === 401 and res.body.code === "UNAUTHORIZED"; keep the
test name or update it to "POST /deviceArtifacts/integrationUpload - Without
auth, should be 401" and keep references to the same endpoint string and test
case (the it(...) block) so CI covers the real POST route.
In `@src/lib/router-utils.ts`:
- Around line 330-338: The call to createArtifactWrappers is using a hardcoded
owner-type string "remediationId" and directly accessing item.artifacts even
though TInputItem doesn't declare it; update the TInputItem definition to
include an optional, correctly typed artifacts array (or add a local
type/interface) and add a runtime type guard (e.g.,
Array.isArray(item.artifacts)) before using it, and determine the correct owner
key dynamically (for example inspect item.remediationId vs item.deviceArtifactId
or add an explicit ownerType discriminator on the input item) then call
createArtifactWrappers(tx, item.artifacts, createdItem.id, ownerKeyName, userId)
with the computed ownerKeyName instead of the hardcoded "remediationId".
- Around line 153-155: Remove the debug comment and the leftover commented
fallback, and make the error handling safe for unknown `e` by replacing the
unsafe `return e.message as string;` with a type-guarded return: check if `e` is
an instance of Error (or has a string `message` property) and return that
message; otherwise return the fallback string "Internal Server Error". Update
the function that contains the `return e.message as string;` line so it never
asserts `unknown` to a string and always returns a safe string value.
---
Nitpick comments:
In `@src/app/api/v1/__tests__/deviceArtifacts.test.ts`:
- Around line 535-543: The cleanup in onTestFinished currently calls
prisma.deviceArtifact.deleteMany({... description: { contains: "mock", mode:
"insensitive" }}) which is too broad and may remove artifacts from other tests;
change the tests that create device artifacts to record the created artifact IDs
(e.g., capture IDs from the integration response or a mapping query in the same
test) and update onTestFinished to delete only those specific IDs using
prisma.deviceArtifact.deleteMany({ where: { id: { in: recordedIds } } }) or
similar; ensure the recordedIds collection is accessible to the teardown
(declare it in the test file scope and push IDs from functions that create
artifacts) and fall back to a safe no-op if the list is empty.
In `@src/features/device-artifacts/server/routers.ts`:
- Around line 220-228: The ts-expect-error on ctx.auth.key arises because the
Context type in src/trpc/init.ts only includes { req } while protectedProcedure
injects an auth property with two flows; update the Context typing (or add a new
ProtectedContext) to explicitly model auth as a discriminated/union type (e.g.,
session flow with key: undefined and api-key flow with key containing id and
userId) so usages like ctx.auth and ctx.auth.key?.id in routers (e.g.,
device-artifacts router where prisma.integration.findFirst and requireExistence
are used) are correctly typed; after adding the new type, switch the router
signatures to use the updated Context/ProtectedContext and remove the
`@ts-expect-error` suppression.
src/lib/router-utils.ts
Outdated
Comment on lines
315
to
328
| await prisma.$transaction(async (tx) => { | ||
| const createdItem = await config.model.create({ | ||
| data: { | ||
| ...createData, | ||
| ...(config.additionalCreateFields?.(userId) || {}), | ||
| externalMappings: { | ||
| create: { | ||
| integrationId, | ||
| externalId: vendorId, | ||
| lastSynced, | ||
| }, | ||
| }, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Get the context around lines 315-328 in the target file
echo "=== Full context from src/lib/router-utils.ts (lines 300-340) ==="
sed -n '300,340p' src/lib/router-utils.ts
echo ""
echo "=== Search for processIntegrationSync function definition ==="
rg -n 'processIntegrationSync' src/lib/router-utils.ts -B2 -A30 --type=ts
echo ""
echo "=== Check how SyncConfig is defined ==="
rg -n 'interface SyncConfig\|type SyncConfig' --type=ts -B2 -A15Repository: PATCH-UPGRADE/viper
Length of output: 2461
🏁 Script executed:
#!/bin/bash
# Search for SyncConfig more broadly
echo "=== Search for SyncConfig definition across all TypeScript files ==="
rg -n 'interface SyncConfig|type SyncConfig|SyncConfig<' --type=ts -B2 -A20 | head -100
echo ""
echo "=== Check imports and where SyncConfig is defined ==="
head -50 src/lib/router-utils.ts
echo ""
echo "=== Look for all uses of config.model within processIntegrationSync ==="
sed -n '233,400p' src/lib/router-utils.ts | grep -n 'config.model'Repository: PATCH-UPGRADE/viper
Length of output: 4646
🏁 Script executed:
#!/bin/bash
# Get the complete SyncConfig definition
echo "=== Full SyncConfig definition (lines 202-230) ==="
sed -n '202,230p' src/lib/router-utils.ts
echo ""
echo "=== Check context around line 130 and 57 ==="
sed -n '50,140p' src/lib/router-utils.ts
echo ""
echo "=== Look for createArtifactWrappers definition ==="
rg -n 'createArtifactWrappers' src/lib/router-utils.ts -B2 -A15
echo ""
echo "=== Check all places where mappingModel is used in transaction context ==="
sed -n '330,360p' src/lib/router-utils.tsRepository: PATCH-UPGRADE/viper
Length of output: 5355
🏁 Script executed:
#!/bin/bash
# Get full context of the transaction blocks and update operations
echo "=== Full context of transaction blocks and updates (lines 260-370) ==="
sed -n '260,370p' src/lib/router-utils.ts
echo ""
echo "=== Check if there are more transaction blocks or model operations after line 370 ==="
sed -n '370,400p' src/lib/router-utils.tsRepository: PATCH-UPGRADE/viper
Length of output: 3608
🏁 Script executed:
#!/bin/bash
# Find all callers of processIntegrationSync to understand the scope
echo "=== All imports and uses of processIntegrationSync ==="
rg -n 'processIntegrationSync' --type=ts -B2 -A5
echo ""
echo "=== Check if TransactionClient is properly typed ==="
rg -n 'type TransactionClient\|interface TransactionClient' --type=ts -B2 -A15Repository: PATCH-UPGRADE/viper
Length of output: 6215
🏁 Script executed:
#!/bin/bash
# Find TransactionClient definition
echo "=== Search for TransactionClient in db.ts ==="
rg -n 'TransactionClient' src/lib/db.ts -B2 -A20
echo ""
echo "=== Check if db.ts exists and its imports ==="
head -80 src/lib/db.ts | cat -nRepository: PATCH-UPGRADE/viper
Length of output: 3342
config.model.create executes outside the interactive transaction — atomicity is broken.
Inside prisma.$transaction(async (tx) => { ... }) (lines 315-339), only operations on tx participate in the transaction. config.model.create(...) at line 316 uses the original non-transactional Prisma delegate, so if createArtifactWrappers(tx, ...) fails after the item is created, the item creation is not rolled back. This breaks atomicity and can leave orphaned records in the database.
To fix this, SyncConfig needs to provide a way to resolve the model delegate from the transaction client:
interface SyncConfig<...> {
model: Pick<PrismaDelegate<TModel>, "findFirst" | "create" | "update">;
+ getTransactionalModel?: (tx: TransactionClient) => Pick<PrismaDelegate<TModel>, "create">;Then use it in the transaction:
await prisma.$transaction(async (tx) => {
- const createdItem = await config.model.create({
+ const txModel = config.getTransactionalModel?.(tx) ?? config.model;
+ const createdItem = await txModel.create({This affects all 4 callers of processIntegrationSync (assets, vulnerabilities, remediations, device-artifacts).
🧰 Tools
🪛 GitHub Check: Lint, TypeScript Type, & Dependency Check
[failure] 315-315:
Parameter 'tx' implicitly has an 'any' type.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/lib/router-utils.ts` around lines 315 - 328, The create call inside
prisma.$transaction is using the standalone config.model.create
(config.model.create) so it runs outside the transaction and breaks atomicity;
update SyncConfig to expose a way to resolve the model delegate from a
transaction client (e.g., a method like getModelFromTx(tx) or withTxModel(tx))
and in processIntegrationSync replace config.model.create with the tx-bound
delegate (use the delegate returned by your new SyncConfig method) so that the
created item and subsequent createArtifactWrappers(tx, ...) calls all run on the
same tx; apply the same change to the other three callers (assets,
vulnerabilities, remediations, device-artifacts) so every model operation inside
prisma.$transaction uses the tx-scoped delegate.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/lib/router-utils.ts (1)
228-228:SyncConfig.artifactWrapperParentIdis dead code — it is never read inprocessIntegrationSync.The optional
artifactWrapperParentIdonSyncConfig(Line 228) is never accessed anywhere in the function body. The value used to drive artifact wrapper creation is always the one returned byconfig.transformInputItem(...)(Line 283). Neither the remediations nor the device-artifacts router sets this field. Remove it to avoid misleading callers.♻️ Proposed fix
// Optional: Additional fields to include in create additionalCreateFields?: (userId: string) => Record<string, any>; - artifactWrapperParentId?: ArtifactWrapperParentIdOptions; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/lib/router-utils.ts` at line 228, Remove the dead optional field artifactWrapperParentId from the SyncConfig type and any related type definitions since processIntegrationSync never reads it; update the SyncConfig interface to delete the artifactWrapperParentId property and remove any tests or docs that mention it, and ensure callers (and implementations of config.transformInputItem) remain unchanged because processIntegrationSync derives the parent ID solely from config.transformInputItem within the processIntegrationSync function.src/app/api/v1/__tests__/remediations.test.ts (1)
497-589: Integration test doesn't verify that artifact wrappers were created.The test validates device groups and sync-status state, but skips verifying that
createArtifactWrappersactually persisted the artifact wrappers (theartifactsinremediationIntegrationPayload.items[*]). The equivalentdeviceArtifacts.test.tstest does assertfoundDeviceArtifact1.artifacts.length === 1. Given that the atomicity of artifact wrapper creation is a known unresolved concern inprocessIntegrationSync, this is the most important path to cover.✅ Suggested additions after Line 548 (and similarly for `foundRem2`)
expect(foundRem1.affectedDeviceGroups[0].cpe).toBe(remPayload1.cpes[0]); + + // Verify artifact wrappers were created + const rem1WithArtifacts = await prisma.remediation.findFirstOrThrow({ + where: { id: mapping1.itemId }, + include: { artifacts: true }, + }); + expect(rem1WithArtifacts.artifacts.length).toBe(remPayload1.artifacts.length);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/api/v1/__tests__/remediations.test.ts` around lines 497 - 589, Add assertions to verify createArtifactWrappers persisted artifacts for each remediation item: after locating foundRem1 and foundRem2, query the artifact-wrapper table (use prisma.remediationArtifact.findFirstOrThrow or the equivalent model used for remediation artifacts) filtering by remediationId === mapping1.itemId and remediationId === mapping2.itemId, then assert the returned records have artifacts.length equal to remediationIntegrationPayload.items[0].artifacts.length and remediationIntegrationPayload.items[1].artifacts.length respectively and that their contents match (e.g., compare first artifact fields); reference the createArtifactWrappers function, remediationIntegrationPayload.items, mapping1.itemId/mapping2.itemId, and prisma.remediationArtifact (or your repo's remediation artifact model) when adding these checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/features/remediations/server/routers.ts`:
- Around line 186-194: The mutation is missing a guard for session-authenticated
users: before calling prisma.integration.findFirst (and similarly in
src/features/device-artifacts/server/routers.ts), check that ctx.auth.key is
defined and has an id, and if not throw/return an authorization error; this
prevents ctx.auth.key?.id from becoming undefined and Prisma from matching a
generic apiKey relation. Update the mutation to explicitly validate
ctx.auth.key.id (e.g., if (!ctx.auth.key?.id) throw new Error("Unauthorized") or
use the project's auth error helper) prior to the prisma.integration.findFirst
call and then continue using requireExistence(integration, "Integration").id as
before.
---
Duplicate comments:
In `@src/app/api/v1/__tests__/deviceArtifacts.test.ts`:
- Around line 128-135: Test is using GET against the POST-only integrationUpload
endpoint; update the test to call .post instead of .get on the request and
adjust the test description to "POST /deviceArtifacts/integrationUpload -
Without auth, should be 401" so it accurately targets the route (locate the
failing test by the string "GET /deviceArtifacts/integrationUpload - Without
auth, should be 401" and change request(BASE_URL).get(...) to
request(BASE_URL).post(...)).
In `@src/app/api/v1/__tests__/remediations.test.ts`:
- Around line 139-144: Test is hitting the wrong HTTP verb: update the test "GET
/remediations/integrationUpload - Without auth, should be 401" in
src/app/api/v1/__tests__/remediations.test.ts so it uses POST instead of GET
when calling request(BASE_URL).get(`/remediations/integrationUpload`); change
that call to request(BASE_URL).post(`/remediations/integrationUpload`) (no auth
or body needed) and keep the existing assertions expecting a 401 and code
"UNAUTHORIZED".
In `@src/features/device-artifacts/server/routers.ts`:
- Around line 220-228: The Prisma findFirst call is vulnerable to session-auth
bypass because ctx.auth.key?.id can be undefined; before calling
prisma.integration.findFirst in the mutation handler, add a guard that checks if
ctx.auth.key?.id is falsy and immediately throw the UNAUTHORIZED error used
elsewhere (same pattern as remediationsRouter.processIntegrationCreate) to
prevent matching integrations when no API key is present; update the logic
around ctx.auth.key?.id, prisma.integration.findFirst, and
requireExistence(integration, "Integration") to rely only on a verified apiKey
id.
In `@src/lib/router-utils.ts`:
- Around line 320-343: The create of the main item uses config.model.create
(global delegate) inside prisma.$transaction, so it is not part of the
transaction and will not roll back if createArtifactWrappers throws; change the
code to use the transactional client (tx) for the model operation — either
add/use a config.getTxModel(tx) (per the SyncConfig suggestion) and call
config.getTxModel(tx).create(...) or otherwise call the appropriate
tx.<modelName>.create(...) instead of config.model.create, and ensure all nested
writes (including externalMappings) and the subsequent call to
createArtifactWrappers(tx, ...) use the same tx to guarantee atomicity.
- Around line 320-338: The transaction callback parameter `tx` is being inferred
as any because `PrismaClientLike.$transaction` is untyped; change the async
callback to explicitly type the parameter as `TransactionClient` (e.g., `async
(tx: TransactionClient) => { ... }`) so `tx` is properly typed for calls like
createArtifactWrappers(tx, ...). Also fix the `item.artifacts` type error by
widening the generic constraint on `TInputItem` to include an optional
`artifacts` property (e.g., `TInputItem extends { vendorId: string; artifacts?:
SomeArtifactInput[] }`) or, if you prefer not to change the generic, add a local
type guard before using `item.artifacts` so calls to createArtifactWrappers only
occur when `item.artifacts` is present; update references to
`createArtifactWrappers`, `config.model.create`, and `prisma.$transaction`
accordingly.
---
Nitpick comments:
In `@src/app/api/v1/__tests__/remediations.test.ts`:
- Around line 497-589: Add assertions to verify createArtifactWrappers persisted
artifacts for each remediation item: after locating foundRem1 and foundRem2,
query the artifact-wrapper table (use
prisma.remediationArtifact.findFirstOrThrow or the equivalent model used for
remediation artifacts) filtering by remediationId === mapping1.itemId and
remediationId === mapping2.itemId, then assert the returned records have
artifacts.length equal to
remediationIntegrationPayload.items[0].artifacts.length and
remediationIntegrationPayload.items[1].artifacts.length respectively and that
their contents match (e.g., compare first artifact fields); reference the
createArtifactWrappers function, remediationIntegrationPayload.items,
mapping1.itemId/mapping2.itemId, and prisma.remediationArtifact (or your repo's
remediation artifact model) when adding these checks.
In `@src/lib/router-utils.ts`:
- Line 228: Remove the dead optional field artifactWrapperParentId from the
SyncConfig type and any related type definitions since processIntegrationSync
never reads it; update the SyncConfig interface to delete the
artifactWrapperParentId property and remove any tests or docs that mention it,
and ensure callers (and implementations of config.transformInputItem) remain
unchanged because processIntegrationSync derives the parent ID solely from
config.transformInputItem within the processIntegrationSync function.
Comment on lines
+186
to
+194
| .mutation(async ({ ctx, input }) => { | ||
| const userId = ctx.auth.user.id; | ||
| const integration = await prisma.integration.findFirst({ | ||
| // @ts-expect-error ctx.auth.key.id is defined if logging in with api key | ||
| where: { apiKey: { id: ctx.auth.key?.id } }, | ||
| select: { id: true }, | ||
| }); | ||
|
|
||
| const integrationId = requireExistence(integration, "Integration").id; |
There was a problem hiding this comment.
Missing API-key guard allows session-authenticated users to trigger syncs against arbitrary integrations.
When ctx.auth.key is undefined (session-based login), ctx.auth.key?.id evaluates to undefined. Prisma silently drops undefined filter values, so where: { apiKey: { id: undefined } } becomes where: { apiKey: {} }, which matches the first integration that has any API key. requireExistence then succeeds, and the endpoint creates remediations and SyncStatus records attributed to that arbitrary integration.
The same pattern is present in src/features/device-artifacts/server/routers.ts at Lines 222–228.
🛡️ Proposed fix — guard before the Prisma query
.mutation(async ({ ctx, input }) => {
const userId = ctx.auth.user.id;
+ const apiKeyId = ctx.auth.key?.id;
+ if (!apiKeyId) {
+ throw new TRPCError({
+ code: "UNAUTHORIZED",
+ message: "This endpoint requires API key authentication",
+ });
+ }
const integration = await prisma.integration.findFirst({
- // `@ts-expect-error` ctx.auth.key.id is defined if logging in with api key
- where: { apiKey: { id: ctx.auth.key?.id } },
+ where: { apiKey: { id: apiKeyId } },
select: { id: true },
});Apply the same fix to src/features/device-artifacts/server/routers.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/features/remediations/server/routers.ts` around lines 186 - 194, The
mutation is missing a guard for session-authenticated users: before calling
prisma.integration.findFirst (and similarly in
src/features/device-artifacts/server/routers.ts), check that ctx.auth.key is
defined and has an id, and if not throw/return an authorization error; this
prevents ctx.auth.key?.id from becoming undefined and Prisma from matching a
generic apiKey relation. Update the mutation to explicitly validate
ctx.auth.key.id (e.g., if (!ctx.auth.key?.id) throw new Error("Unauthorized") or
use the project's auth error helper) prior to the prisma.integration.findFirst
call and then continue using requireExistence(integration, "Integration").id as
before.
added 2 commits
February 19, 2026 16:01
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.
Summary by CodeRabbit
New Features
Bug Fixes
Tests