Skip to content

Commit 699b984

Browse files
committed
improvement(auth): bind sealed tokens to their column and guard the backfill key
1 parent b3b6b45 commit 699b984

15 files changed

Lines changed: 511 additions & 77 deletions

‎apps/sim/instrumentation-node.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,9 @@ export async function register() {
378378

379379
await initializeOpenTelemetry()
380380

381+
const { reportAccountTokenSealing } = await import('@sim/db/sealed-text')
382+
reportAccountTokenSealing()
383+
381384
const shutdownPostHog = async () => {
382385
try {
383386
const { getPostHogClient } = await import('@/lib/posthog/server')

‎apps/sim/lib/core/config/env.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ export const env = createEnv({
142142
TURNSTILE_SECRET_KEY: z.string().min(1).optional(), // Cloudflare Turnstile secret key for captcha verification
143143
ENCRYPTION_KEY: z.string().min(32), // Key for encrypting sensitive data
144144
API_ENCRYPTION_KEY: z.string().min(32).optional(), // Dedicated key for encrypting API keys (optional for OSS)
145-
ENCRYPT_ACCOUNT_TOKENS: z.boolean().optional(), // Seal OAuth access/refresh tokens with ENCRYPTION_KEY on write (read by @sim/db). Enable only once every app, worker, and background-task deployment runs a release that reads sealed tokens
145+
ENCRYPT_ACCOUNT_TOKENS: z.boolean().optional(), // Seal OAuth access/refresh tokens on write (read by @sim/db); takes effect only with a 64-hex ENCRYPTION_KEY. Enable only once every app, worker, and background-task deployment runs a release that reads sealed tokens
146146
INTERNAL_API_SECRET: z.string().min(32), // Secret for internal API authentication
147147
INTERNAL_JWT_SECRET: z.string().min(32).optional(), // Dedicated signing key for internal JWTs (falls back to INTERNAL_API_SECRET); separating limits blast radius if one leaks
148148

‎apps/sim/lib/oauth/credential-revocation.integration.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import { fanOutSlackTokenChain } from '@/lib/oauth/slack'
3636
import { completeOAuthCredentialToken } from '@/lib/oauth/token-resolution'
3737

3838
const originalFetch = globalThis.fetch
39+
const fixtureEncryptionKey = process.env.ENCRYPTION_KEY
3940
const userId = generateId()
4041
const workspaceId = generateId()
4142
const accountId = generateId()
@@ -189,6 +190,7 @@ beforeEach(async () => {
189190

190191
afterEach((context) => {
191192
sealWrites(false)
193+
process.env.ENCRYPTION_KEY = fixtureEncryptionKey
192194
pausedProvider?.release.resolve()
193195
checks.push({
194196
name: context.task.name,
@@ -488,6 +490,15 @@ describe('OAuth refresh chains with sealed account tokens', () => {
488490
})
489491
})
490492

493+
it('keeps writing readable tokens when sealing is requested without a usable key', async () => {
494+
sealWrites(true)
495+
process.env.ENCRYPTION_KEY = 'not-hex'.repeat(6)
496+
providerAnswer = REFRESHED
497+
498+
await expect(resolveToken()).resolves.toEqual({ accessToken: 'fresh-access', refreshed: true })
499+
expect(await rawTokens()).toMatchObject({ access_token: 'fresh-access' })
500+
})
501+
491502
it('rotates a chain stored before sealing was enabled', async () => {
492503
sealWrites(true)
493504
providerAnswer = REFRESHED

‎apps/sim/lib/oauth/credential-service.test.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,11 @@ const serviceLogger = getMockLogger('OAuthCredentialService')
8686
const transportLogger = getMockLogger('GoogleServiceAccountTransport')
8787
const mocks = { ...hoisted, decryptSecret: encryptionMockFns.mockDecryptSecret }
8888

89+
beforeEach(() => {
90+
// Schema mocks carry no table to seal against; credential-revocation.integration.ts covers sealing.
91+
vi.stubEnv('ENCRYPTION_KEY', '')
92+
})
93+
8994
/** Every call the credential service and its token transport logged at the given levels. */
9095
function loggedCalls(...levels: Array<'info' | 'warn' | 'error'>) {
9196
return levels.flatMap((level) => [

‎apps/sim/trigger.config.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,8 @@ export default defineConfig({
133133
createLogger('TriggerInit').error('Could not distrust compromised CA roots', tlsTrust)
134134
}
135135
markInsideTriggerRun()
136+
const { reportAccountTokenSealing } = await import('@sim/db/sealed-text')
137+
reportAccountTokenSealing()
136138
const { warmRedisConnection } = await import('./lib/core/config/redis')
137139
await warmRedisConnection()
138140
},

‎package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@
7676
"check:spec-example-ids": "bun run scripts/check-spec-example-ids.ts",
7777
"check:script-tests": "bun run scripts/check-script-test-coverage.ts",
7878
"check:test-patterns": "bun run scripts/check-test-patterns.ts",
79+
"check:sealed-columns": "bun run scripts/check-sealed-columns.ts",
7980
"check:schema-mock": "bun run scripts/generate-schema-mock.ts --check",
8081
"check:audits": "bun run scripts/run-audits.ts",
8182
"check:skills": "bun run scripts/sync-skills.ts --check",

‎packages/db/schema.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,8 @@ export const account = pgTable(
177177
userId: text('user_id')
178178
.notNull()
179179
.references(() => user.id, { onDelete: 'cascade' }),
180-
accessToken: sealedText('access_token'),
181-
refreshToken: sealedText('refresh_token'),
180+
accessToken: sealedText('account', 'access_token'),
181+
refreshToken: sealedText('account', 'refresh_token'),
182182
idToken: text('id_token'),
183183
accessTokenExpiresAt: timestamp('access_token_expires_at'),
184184
refreshTokenExpiresAt: timestamp('refresh_token_expires_at'),

‎packages/db/script-migrations/0037_seal_account_tokens.integration.ts‎

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
import { sealAccountTokens } from '@sim/db/script-migrations/0037_seal_account_tokens'
22
import { openText } from '@sim/db/sealed-text'
33
import { readTestDatabaseUrl } from '@sim/db/testing/test-infrastructure'
4+
import { seal, sealingKeyId } from '@sim/security/encryption'
45
import { generateId } from '@sim/utils/id'
56
import postgres, { type Sql } from 'postgres'
67
import { afterAll, beforeAll, beforeEach, describe, expect, it } from 'vitest'
78

89
const databaseUrl = readTestDatabaseUrl()
10+
const KEY_HEX = 'a'.repeat(64)
11+
const KEY_ID = sealingKeyId(Buffer.from(KEY_HEX, 'hex'))
12+
const ACCESS_CONTEXT = 'account.access_token'
13+
const REFRESH_CONTEXT = 'account.refresh_token'
914

1015
interface StoredRow {
1116
id: string
@@ -22,7 +27,7 @@ describe('sealing stored account tokens in PostgreSQL', () => {
2227
const previousKey = process.env.ENCRYPTION_KEY
2328

2429
beforeAll(async () => {
25-
process.env.ENCRYPTION_KEY = 'a'.repeat(64)
30+
process.env.ENCRYPTION_KEY = KEY_HEX
2631
admin = postgres(databaseUrl, { max: 1, onnotice: () => undefined })
2732
await admin.unsafe(`CREATE SCHEMA "${schemaName}"`)
2833
await admin.unsafe(
@@ -69,37 +74,55 @@ describe('sealing stored account tokens in PostgreSQL', () => {
6974
const accessOnly = await insertAccount('access-only', null)
7075
const before = await storedRows()
7176

72-
expect(await sealAccountTokens(sql, { mode: 'seal', batchSize: 2, pauseMs: 0 })).toEqual({
77+
expect(
78+
await sealAccountTokens(sql, {
79+
mode: 'seal',
80+
expectedKeyId: KEY_ID,
81+
batchSize: 2,
82+
pauseMs: 0,
83+
})
84+
).toEqual({
85+
keyId: KEY_ID,
7386
scanned: 6,
7487
pending: 6,
7588
updated: 6,
89+
foreignKey: 0,
7690
})
7791

7892
const after = await storedRows()
7993
for (const row of after) {
8094
const original = before.find((candidate) => candidate.id === row.id)
8195
expect(row.updated_at).toBe(original?.updated_at)
8296
expect(row.access_token).toMatch(/^enc:v1:/)
83-
expect(openText(row.access_token ?? '')).toBe(original?.access_token)
97+
expect(openText(row.access_token ?? '', ACCESS_CONTEXT)).toBe(original?.access_token)
8498
if (row.id === accessOnly) {
8599
expect(row.refresh_token).toBeNull()
86100
} else {
87101
expect(row.refresh_token).toMatch(/^enc:v1:/)
88-
expect(openText(row.refresh_token ?? '')).toBe(original?.refresh_token)
102+
expect(openText(row.refresh_token ?? '', REFRESH_CONTEXT)).toBe(original?.refresh_token)
89103
}
90104
}
91105
})
92106

93107
it('finds nothing left to do on a rerun or a dry run after a completed pass', async () => {
94108
await insertAccount('access', 'refresh')
95-
await sealAccountTokens(sql, { mode: 'seal', pauseMs: 0 })
109+
await sealAccountTokens(sql, { mode: 'seal', expectedKeyId: KEY_ID, pauseMs: 0 })
96110
const sealed = await storedRows()
97111

98-
expect(await sealAccountTokens(sql, { mode: 'seal', pauseMs: 0 })).toMatchObject({
112+
expect(
113+
await sealAccountTokens(sql, { mode: 'seal', expectedKeyId: KEY_ID, pauseMs: 0 })
114+
).toMatchObject({
99115
pending: 0,
100116
updated: 0,
101117
})
102-
expect(await sealAccountTokens(sql, { mode: 'seal', dryRun: true, pauseMs: 0 })).toMatchObject({
118+
expect(
119+
await sealAccountTokens(sql, {
120+
mode: 'seal',
121+
expectedKeyId: KEY_ID,
122+
dryRun: true,
123+
pauseMs: 0,
124+
})
125+
).toMatchObject({
103126
pending: 0,
104127
})
105128
expect(await storedRows()).toEqual(sealed)
@@ -109,10 +132,19 @@ describe('sealing stored account tokens in PostgreSQL', () => {
109132
await insertAccount('access', 'refresh')
110133
const before = await storedRows()
111134

112-
expect(await sealAccountTokens(sql, { mode: 'seal', dryRun: true, pauseMs: 0 })).toEqual({
135+
expect(
136+
await sealAccountTokens(sql, {
137+
mode: 'seal',
138+
expectedKeyId: KEY_ID,
139+
dryRun: true,
140+
pauseMs: 0,
141+
})
142+
).toEqual({
143+
keyId: KEY_ID,
113144
scanned: 1,
114145
pending: 1,
115146
updated: 0,
147+
foreignKey: 0,
116148
})
117149
expect(await storedRows()).toEqual(before)
118150
})
@@ -121,11 +153,47 @@ describe('sealing stored account tokens in PostgreSQL', () => {
121153
await insertAccount('ya29.access:with:colons', 'refresh')
122154
await insertAccount(null, 'refresh-only')
123155
const plaintext = await storedRows()
124-
await sealAccountTokens(sql, { mode: 'seal', pauseMs: 0 })
156+
await sealAccountTokens(sql, { mode: 'seal', expectedKeyId: KEY_ID, pauseMs: 0 })
125157

126-
expect(await sealAccountTokens(sql, { mode: 'unseal', pauseMs: 0 })).toMatchObject({
158+
expect(
159+
await sealAccountTokens(sql, { mode: 'unseal', expectedKeyId: KEY_ID, pauseMs: 0 })
160+
).toMatchObject({
127161
updated: 2,
128162
})
129163
expect(await storedRows()).toEqual(plaintext)
130164
})
165+
166+
it('refuses to start under a key the fleet does not use, before touching any row', async () => {
167+
await insertAccount('access', 'refresh')
168+
const before = await storedRows()
169+
170+
for (const dryRun of [false, true]) {
171+
await expect(
172+
sealAccountTokens(sql, { mode: 'seal', expectedKeyId: 'ffffffff', dryRun, pauseMs: 0 })
173+
).rejects.toThrow(KEY_ID)
174+
}
175+
await expect(sealAccountTokens(sql, { mode: 'seal', pauseMs: 0 })).rejects.toThrow()
176+
expect(await storedRows()).toEqual(before)
177+
})
178+
179+
it('skips and reports rows sealed under another key instead of failing the pass', async () => {
180+
const otherKey = Buffer.from('b'.repeat(64), 'hex')
181+
const foreignAccess = seal('foreign-access', otherKey, ACCESS_CONTEXT)
182+
const foreign = await insertAccount(foreignAccess, 'foreign-refresh')
183+
await insertAccount('own-access', 'own-refresh')
184+
await sealAccountTokens(sql, { mode: 'seal', expectedKeyId: KEY_ID, pauseMs: 0 })
185+
186+
expect(
187+
await sealAccountTokens(sql, { mode: 'unseal', expectedKeyId: KEY_ID, pauseMs: 0 })
188+
).toMatchObject({ scanned: 2, updated: 1, foreignKey: 1 })
189+
const rows = await storedRows()
190+
expect(rows.find((row) => row.id === foreign)).toMatchObject({
191+
access_token: foreignAccess,
192+
refresh_token: 'foreign-refresh',
193+
})
194+
expect(rows.find((row) => row.id !== foreign)).toMatchObject({
195+
access_token: 'own-access',
196+
refresh_token: 'own-refresh',
197+
})
198+
})
131199
})

‎packages/db/script-migrations/0037_seal_account_tokens.ts‎

Lines changed: 51 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,20 @@
1-
import { openText, sealText } from '@sim/db/sealed-text'
1+
import { account } from '@sim/db/schema'
2+
import { openText, readEncryptionKey, sealedTextContext, sealText } from '@sim/db/sealed-text'
23
import { createLogger } from '@sim/logger'
4+
import { sealedKeyId, sealingKeyId } from '@sim/security/encryption'
35
import { sleep } from '@sim/utils/helpers'
46
import type { Sql } from 'postgres'
57

6-
const logger = createLogger('AccountTokenSealing')
8+
const logger = createLogger('AccountTokenSealing', { enabled: true, logLevel: 'INFO' })
79

810
interface AccountTokenSealingOptions {
911
mode: 'seal' | 'unseal'
12+
/**
13+
* The id the fleet logs at startup (`Account token sealing`). A pass refuses to start under any
14+
* other `ENCRYPTION_KEY`, so it can never seal rows with a key the fleet cannot open. Optional
15+
* only for a dry run.
16+
*/
17+
expectedKeyId?: string
1018
/** Counts the rows that would change without writing any. */
1119
dryRun?: boolean
1220
batchSize?: number
@@ -15,9 +23,12 @@ interface AccountTokenSealingOptions {
1523
}
1624

1725
interface AccountTokenSealingResult {
26+
keyId: string
1827
scanned: number
1928
pending: number
2029
updated: number
30+
/** Rows left untouched because a token is sealed under another key. */
31+
foreignKey: number
2132
}
2233

2334
interface StoredTokens {
@@ -26,20 +37,46 @@ interface StoredTokens {
2637
refresh_token: string | null
2738
}
2839

40+
const ACCESS_TOKEN_CONTEXT = sealedTextContext(account.accessToken)
41+
const REFRESH_TOKEN_CONTEXT = sealedTextContext(account.refreshToken)
42+
2943
/**
3044
* Rewrites `account.access_token` and `account.refresh_token` into the sealed form (`seal`) or back
3145
* to plaintext (`unseal`, the rollback). Idempotent and resumable: a value already in the target
3246
* form is skipped, so a rerun only finishes what an interrupted run left. Each row is rewritten only
3347
* while it still holds the value this pass read, so a refresh that rotates the chain concurrently
34-
* wins, and `updated_at` is left alone because the refresh path reads it as the chain version.
48+
* wins, and `updated_at` is left alone because the refresh path reads it as the chain version. Rows
49+
* sealed under another key are skipped and counted rather than failing the pass.
3550
*/
3651
export async function sealAccountTokens(
3752
sql: Sql,
38-
{ mode, dryRun = false, batchSize = 200, pauseMs = 100 }: AccountTokenSealingOptions
53+
{
54+
mode,
55+
expectedKeyId,
56+
dryRun = false,
57+
batchSize = 200,
58+
pauseMs = 100,
59+
}: AccountTokenSealingOptions
3960
): Promise<AccountTokenSealingResult> {
40-
const convert = (value: string | null) =>
41-
value === null ? null : mode === 'seal' ? sealText(value) : openText(value)
42-
const result: AccountTokenSealingResult = { scanned: 0, pending: 0, updated: 0 }
61+
const keyId = sealingKeyId(readEncryptionKey())
62+
if ((!dryRun || expectedKeyId !== undefined) && expectedKeyId !== keyId) {
63+
throw new Error(
64+
`ENCRYPTION_KEY has key id ${keyId}, not the expected ${expectedKeyId ?? '(none)'}`
65+
)
66+
}
67+
const isForeign = (value: string | null) => {
68+
const sealedUnder = value === null ? null : sealedKeyId(value)
69+
return sealedUnder !== null && sealedUnder !== keyId
70+
}
71+
const convert = (value: string | null, context: string) =>
72+
value === null ? null : mode === 'seal' ? sealText(value, context) : openText(value, context)
73+
const result: AccountTokenSealingResult = {
74+
keyId,
75+
scanned: 0,
76+
pending: 0,
77+
updated: 0,
78+
foreignKey: 0,
79+
}
4380
let afterId = ''
4481

4582
for (;;) {
@@ -52,11 +89,15 @@ export async function sealAccountTokens(
5289
afterId = rows[rows.length - 1].id
5390
result.scanned += rows.length
5491

55-
const changes = rows
92+
const ownRows = rows.filter(
93+
(row) => !isForeign(row.access_token) && !isForeign(row.refresh_token)
94+
)
95+
result.foreignKey += rows.length - ownRows.length
96+
const changes = ownRows
5697
.map((row) => ({
5798
row,
58-
accessToken: convert(row.access_token),
59-
refreshToken: convert(row.refresh_token),
99+
accessToken: convert(row.access_token, ACCESS_TOKEN_CONTEXT),
100+
refreshToken: convert(row.refresh_token, REFRESH_TOKEN_CONTEXT),
60101
}))
61102
.filter(
62103
({ row, accessToken, refreshToken }) =>

0 commit comments

Comments
 (0)