Skip to content

Commit 59d6b8a

Browse files
waleedlatif1waleedlatif1
authored
fix(onepassword): validate integration against API docs, add file downloads (#5365)
* fix(onepassword): validate integration against API docs, add file downloads - add onepassword_get_item_file tool + route for downloading item file attachments (SDK items.files.read / Connect files/{id}/content), backed by newly-exposed item.files metadata on get/create/replace/update item - fix update_item JSON Patch applying array indices instead of 1Password's documented field-ID addressing (/fields/{fieldId}/...), which silently dropped field edits in Service Account mode - fix Service Account mode's list-vaults/list-items filter to honor SCIM `eq` exact-match semantics instead of always substring-matching - expand the create-item category dropdown from 9 to 19 real, creatable 1Password categories (was missing SOFTWARE_LICENSE, EMAIL_ACCOUNT, MEMBERSHIP, PASSPORT, REWARD_PROGRAM, DRIVER_LICENSE, BANK_ACCOUNT, MEDICAL_RECORD, OUTDOOR_LICENSE, WIRELESS_ROUTER, SOCIAL_SECURITY_NUMBER) - replace the block's single opaque `response: json` output with typed, per-operation output fields matching repo convention - remove incorrect password-masking on the Vault ID field (not a secret) - re-export tool types from the onepassword barrel * fix(onepassword): honor SCIM attribute name in filter matcher matchesFilter always compared against name/title regardless of the attribute named in the eq expression, so `id eq "..."` incorrectly matched against the display name instead of the id. * fix(onepassword): close output-parity and doc-string gaps from final audit - restore a deprecated no-op 'response' output so pre-existing saved workflows referencing it fail soft (empty) instead of hard-erroring now that per-operation outputs replace it - add missing block outputs (urls, favorite, version, state, lastEditedBy) for get/create/replace/update item so all real FULL_ITEM fields are discoverable as <Block.field> references - hide Connect Server credential fields for Resolve Secret (Service Account only) instead of leaving them selectable and silently ignored - correct two doc-string enum lists that advertised values the API doesn't return (vault type TRANSFER, item state DELETED) * fix(onepassword): fix silent data loss in update_item (Service Account mode) update_item applied user JSON Patch ops (documented/typed against the Connect-shaped vocabulary get_item returns: label/type/section.id) directly onto the raw SDK item, whose vocabulary differs (title/ fieldType/sectionId, and SDK category enum strings vs Connect's SCREAMING_SNAKE_CASE). Most patches beyond /title, /tags/-, and /fields/{id}/value silently no-opped or could corrupt the item while still reporting success. Extracted the Connect->SDK item conversion already used by replace_item into a shared connectItemToSdkItem helper. update_item now normalizes the fetched item to Connect shape, applies patches to that, then converts back before calling items.put() -- matching create/replace's existing translation pattern. Found via an adversarial final-verification pass that traced concrete patch operations by hand against the SDK's actual field vocabulary. * fix(onepassword): preserve field metadata and empty-title fallback connectItemToSdkItem rebuilt every field as a bare object, dropping SDK-only metadata (e.g. password-generation details) that a raw patch/replace previously left untouched. Now merges onto the existing SDK field by id before applying the translated properties, and only starts fields bare when they're genuinely new. Also restored the || (not ??) fallback on title to match replace_item's prior behavior of treating an explicitly empty title as "not provided".
1 parent f658e6d commit 59d6b8a

18 files changed

Lines changed: 695 additions & 69 deletions

File tree

‎apps/sim/app/api/tools/onepassword/get-item-file/route.ts‎

Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import { createLogger } from '@sim/logger'
2+
import { getErrorMessage } from '@sim/utils/errors'
3+
import { generateId } from '@sim/utils/id'
4+
import { type NextRequest, NextResponse } from 'next/server'
5+
import { onePasswordGetItemFileContract } from '@/lib/api/contracts/tools/onepassword'
6+
import { parseRequest, validationErrorResponse } from '@/lib/api/server'
7+
import { checkInternalAuth } from '@/lib/auth/hybrid'
8+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
9+
import {
10+
connectRequest,
11+
createOnePasswordClient,
12+
findItemFileAttributes,
13+
resolveCredentials,
14+
} from '../utils'
15+
16+
const logger = createLogger('OnePasswordGetItemFileAPI')
17+
18+
export const POST = withRouteHandler(async (request: NextRequest) => {
19+
const requestId = generateId().slice(0, 8)
20+
21+
const auth = await checkInternalAuth(request)
22+
if (!auth.success || !auth.userId) {
23+
logger.warn(`[${requestId}] Unauthorized 1Password get-item-file attempt`)
24+
return NextResponse.json({ error: auth.error || 'Unauthorized' }, { status: 401 })
25+
}
26+
27+
try {
28+
const parsed = await parseRequest(
29+
onePasswordGetItemFileContract,
30+
request,
31+
{},
32+
{
33+
validationErrorResponse: (error) => validationErrorResponse(error, 'Invalid request data'),
34+
}
35+
)
36+
if (!parsed.success) return parsed.response
37+
const params = parsed.data.body
38+
const creds = resolveCredentials(params)
39+
40+
logger.info(
41+
`[${requestId}] Downloading file ${params.fileId} from item ${params.itemId} (${creds.mode} mode)`
42+
)
43+
44+
if (creds.mode === 'service_account') {
45+
const client = await createOnePasswordClient(creds.serviceAccountToken!)
46+
const item = await client.items.get(params.vaultId, params.itemId)
47+
const attr = findItemFileAttributes(item, params.fileId)
48+
if (!attr) {
49+
return NextResponse.json({ error: 'File not found on item' }, { status: 404 })
50+
}
51+
52+
const content = await client.items.files.read(params.vaultId, params.itemId, attr)
53+
return NextResponse.json({
54+
file: {
55+
name: attr.name,
56+
mimeType: 'application/octet-stream',
57+
data: Buffer.from(content).toString('base64'),
58+
size: attr.size,
59+
},
60+
})
61+
}
62+
63+
const metaResponse = await connectRequest({
64+
serverUrl: creds.serverUrl!,
65+
apiKey: creds.apiKey!,
66+
path: `/v1/vaults/${params.vaultId}/items/${params.itemId}/files/${params.fileId}`,
67+
method: 'GET',
68+
})
69+
if (!metaResponse.ok) {
70+
const metaData = await metaResponse.json().catch(() => ({}))
71+
return NextResponse.json(
72+
{ error: metaData.message || 'Failed to get file metadata' },
73+
{ status: metaResponse.status }
74+
)
75+
}
76+
const meta = await metaResponse.json()
77+
78+
const contentResponse = await connectRequest({
79+
serverUrl: creds.serverUrl!,
80+
apiKey: creds.apiKey!,
81+
path: `/v1/vaults/${params.vaultId}/items/${params.itemId}/files/${params.fileId}/content`,
82+
method: 'GET',
83+
})
84+
if (!contentResponse.ok) {
85+
const errorData = await contentResponse.json().catch(() => ({}))
86+
return NextResponse.json(
87+
{ error: errorData.message || 'Failed to download file content' },
88+
{ status: contentResponse.status }
89+
)
90+
}
91+
92+
const buffer = Buffer.from(await contentResponse.arrayBuffer())
93+
return NextResponse.json({
94+
file: {
95+
name: meta.name ?? 'attachment',
96+
mimeType: contentResponse.headers.get('content-type') || 'application/octet-stream',
97+
data: buffer.toString('base64'),
98+
size: meta.size ?? buffer.length,
99+
},
100+
})
101+
} catch (error) {
102+
const message = getErrorMessage(error, 'Unknown error')
103+
logger.error(`[${requestId}] Get item file failed:`, error)
104+
return NextResponse.json({ error: `Failed to get item file: ${message}` }, { status: 500 })
105+
}
106+
})

‎apps/sim/app/api/tools/onepassword/list-items/route.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import {
1010
connectRequest,
1111
createOnePasswordClient,
12+
matchesFilter,
1213
normalizeSdkItemOverview,
1314
resolveCredentials,
1415
} from '../utils'
@@ -45,11 +46,9 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4546
const normalized = items.map(normalizeSdkItemOverview)
4647

4748
if (params.filter) {
48-
const filterLower = params.filter.toLowerCase()
49-
const filtered = normalized.filter(
50-
(item) =>
51-
item.title?.toLowerCase().includes(filterLower) ||
52-
item.id?.toLowerCase().includes(filterLower)
49+
const filter = params.filter
50+
const filtered = normalized.filter((item) =>
51+
matchesFilter(item.title ?? '', item.id ?? '', filter)
5352
)
5453
return NextResponse.json(filtered)
5554
}

‎apps/sim/app/api/tools/onepassword/list-vaults/route.ts‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import {
1010
connectRequest,
1111
createOnePasswordClient,
12+
matchesFilter,
1213
normalizeSdkVault,
1314
resolveCredentials,
1415
} from '../utils'
@@ -45,11 +46,8 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4546
const normalized = vaults.map(normalizeSdkVault)
4647

4748
if (params.filter) {
48-
const filterLower = params.filter.toLowerCase()
49-
const filtered = normalized.filter(
50-
(v) =>
51-
v.name?.toLowerCase().includes(filterLower) || v.id?.toLowerCase().includes(filterLower)
52-
)
49+
const filter = params.filter
50+
const filtered = normalized.filter((v) => matchesFilter(v.name ?? '', v.id ?? '', filter))
5351
return NextResponse.json(filtered)
5452
}
5553

‎apps/sim/app/api/tools/onepassword/replace-item/route.ts‎

Lines changed: 2 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import type { Item } from '@1password/sdk'
21
import { createLogger } from '@sim/logger'
32
import { getErrorMessage } from '@sim/utils/errors'
43
import { generateId } from '@sim/utils/id'
@@ -8,12 +7,11 @@ import { parseRequest, validationErrorResponse } from '@/lib/api/server'
87
import { checkInternalAuth } from '@/lib/auth/hybrid'
98
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
109
import {
10+
connectItemToSdkItem,
1111
connectRequest,
1212
createOnePasswordClient,
1313
normalizeSdkItem,
1414
resolveCredentials,
15-
toSdkCategory,
16-
toSdkFieldType,
1715
} from '../utils'
1816

1917
const logger = createLogger('OnePasswordReplaceItemAPI')
@@ -49,40 +47,7 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4947
const client = await createOnePasswordClient(creds.serviceAccountToken!)
5048

5149
const existing = await client.items.get(params.vaultId, params.itemId)
52-
53-
const sdkItem = {
54-
...existing,
55-
id: params.itemId,
56-
title: itemData.title || existing.title,
57-
category: itemData.category ? toSdkCategory(itemData.category) : existing.category,
58-
vaultId: params.vaultId,
59-
fields: itemData.fields
60-
? (itemData.fields as Array<Record<string, any>>).map((f) => ({
61-
id: f.id || generateId().slice(0, 8),
62-
title: f.label || f.title || '',
63-
fieldType: toSdkFieldType(f.type || 'STRING'),
64-
value: f.value || '',
65-
sectionId: f.section?.id ?? f.sectionId,
66-
}))
67-
: existing.fields,
68-
sections: itemData.sections
69-
? (itemData.sections as Array<Record<string, any>>).map((s) => ({
70-
id: s.id || '',
71-
title: s.label || s.title || '',
72-
}))
73-
: existing.sections,
74-
notes: itemData.notes ?? existing.notes,
75-
tags: itemData.tags ?? existing.tags,
76-
websites:
77-
itemData.urls || itemData.websites
78-
? (itemData.urls ?? itemData.websites ?? []).map((u: Record<string, any>) => ({
79-
url: u.href || u.url || '',
80-
label: u.label || '',
81-
autofillBehavior: 'AnywhereOnWebsite' as const,
82-
}))
83-
: existing.websites,
84-
} as Item
85-
50+
const sdkItem = connectItemToSdkItem(itemData, existing)
8651
const result = await client.items.put(sdkItem)
8752
return NextResponse.json(normalizeSdkItem(result))
8853
}

‎apps/sim/app/api/tools/onepassword/update-item/route.ts‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { parseRequest, validationErrorResponse } from '@/lib/api/server'
77
import { checkInternalAuth } from '@/lib/auth/hybrid'
88
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
99
import {
10+
connectItemToSdkItem,
1011
connectRequest,
1112
createOnePasswordClient,
1213
normalizeSdkItem,
@@ -45,13 +46,21 @@ export const POST = withRouteHandler(async (request: NextRequest) => {
4546
if (creds.mode === 'service_account') {
4647
const client = await createOnePasswordClient(creds.serviceAccountToken!)
4748

48-
const item = await client.items.get(params.vaultId, params.itemId)
49+
const existing = await client.items.get(params.vaultId, params.itemId)
4950

51+
// Patch operations are documented and typed against the Connect-shaped
52+
// vocabulary (label/type/section.id) that get_item/create_item/replace_item
53+
// return — apply them to that normalized view, then convert back to the
54+
// SDK's vocabulary (title/fieldType/sectionId) before writing. Patching the
55+
// raw SDK item directly would silently no-op most field/category writes.
56+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
57+
const connectItem = normalizeSdkItem(existing) as Record<string, any>
5058
for (const op of ops) {
51-
applyPatch(item, op)
59+
applyPatch(connectItem, op)
5260
}
5361

54-
const result = await client.items.put(item)
62+
const sdkItem = connectItemToSdkItem(connectItem, existing)
63+
const result = await client.items.put(sdkItem)
5564
return NextResponse.json(normalizeSdkItem(result))
5665
}
5766

@@ -104,7 +113,7 @@ function applyPatch(item: Record<string, any>, op: JsonPatchOperation) {
104113
for (let i = 0; i < segments.length - 1; i++) {
105114
const seg = segments[i]
106115
if (Array.isArray(target)) {
107-
target = target[Number(seg)]
116+
target = arrayElementForSegment(target, seg)
108117
} else {
109118
target = target[seg]
110119
}
@@ -117,15 +126,37 @@ function applyPatch(item: Record<string, any>, op: JsonPatchOperation) {
117126
if (Array.isArray(target) && lastSeg === '-') {
118127
target.push(op.value)
119128
} else if (Array.isArray(target)) {
120-
target[Number(lastSeg)] = op.value
129+
const index = arrayIndexForSegment(target, lastSeg)
130+
if (index !== -1) target[index] = op.value
121131
} else {
122132
target[lastSeg] = op.value
123133
}
124134
} else if (op.op === 'remove') {
125135
if (Array.isArray(target)) {
126-
target.splice(Number(lastSeg), 1)
136+
const index = arrayIndexForSegment(target, lastSeg)
137+
if (index !== -1) target.splice(index, 1)
127138
} else {
128139
delete target[lastSeg]
129140
}
130141
}
131142
}
143+
144+
/**
145+
* Resolves an array element for a JSON Patch path segment. 1Password's PATCH API
146+
* addresses items in the `fields`/`sections` arrays by their `id`, not by numeric
147+
* array index (e.g. `/fields/{fieldId}/value`), so a numeric-looking segment is
148+
* only treated as a literal index when no element's `id` matches it.
149+
*/
150+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
151+
function arrayIndexForSegment(target: any[], segment: string): number {
152+
const byId = target.findIndex((el) => el && typeof el === 'object' && el.id === segment)
153+
if (byId !== -1) return byId
154+
const index = Number(segment)
155+
return Number.isInteger(index) && index >= 0 && index < target.length ? index : -1
156+
}
157+
158+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
159+
function arrayElementForSegment(target: any[], segment: string): any {
160+
const index = arrayIndexForSegment(target, segment)
161+
return index === -1 ? undefined : target[index]
162+
}

0 commit comments

Comments
 (0)