-
Notifications
You must be signed in to change notification settings - Fork 20
fix(backend): connection-config encryption round-trip — align encrypt/decrypt prefix #464
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
buremba
merged 1 commit into
feat/owletto-cli-merge
from
fix/connection-encryption-asymmetry
Apr 30, 2026
+174
−6
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
69 changes: 69 additions & 0 deletions
69
db/migrations/20260430022231_fix_connection_config_encryption.sql
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| -- migrate:up | ||
|
|
||
| -- Fix connection-config encryption asymmetry in agent_connections.config. | ||
| -- | ||
| -- encryptConfig() in postgres-stores.ts historically returned raw | ||
| -- "iv:tag:ciphertext" output from @lobu/core's `encrypt()`, but | ||
| -- decryptConfig() only decrypts strings that start with "enc:v1:". So any | ||
| -- secret-named field that hit encryptConfig was stored as prefixless | ||
| -- ciphertext and round-tripped as that ciphertext literal on read. | ||
| -- | ||
| -- This migration backfills existing prefixless rows by re-prefixing them so | ||
| -- the now-aligned decryptConfig path can decrypt them. | ||
| -- | ||
| -- Identification: AES-GCM in @lobu/core uses a 12-byte IV (24 hex chars) | ||
| -- and a 16-byte auth tag (32 hex chars), joined with the ciphertext as | ||
| -- `iv:tag:ciphertext`. We match exactly that shape to avoid touching | ||
| -- arbitrary `:` separated values. | ||
| -- | ||
| -- Idempotent: jsonb_object_agg only rewrites string values that match the | ||
| -- prefixless shape AND lack the prefix. Re-running the migration is a noop. | ||
|
|
||
| UPDATE public.agent_connections AS ac | ||
| SET config = sub.fixed_config | ||
| FROM ( | ||
| SELECT | ||
| id, | ||
| jsonb_object_agg( | ||
| key, | ||
| CASE | ||
| WHEN jsonb_typeof(value) = 'string' | ||
| AND value #>> '{}' ~ '^[0-9a-f]{24}:[0-9a-f]{32}:[0-9a-f]+$' | ||
| AND value #>> '{}' NOT LIKE 'enc:v1:%' | ||
| THEN to_jsonb('enc:v1:' || (value #>> '{}')) | ||
| ELSE value | ||
| END | ||
| ) AS fixed_config | ||
| FROM public.agent_connections, | ||
| LATERAL jsonb_each(config) | ||
| GROUP BY id | ||
| ) AS sub | ||
| WHERE ac.id = sub.id | ||
| AND ac.config IS DISTINCT FROM sub.fixed_config; | ||
|
|
||
| -- migrate:down | ||
|
|
||
| -- Strip the "enc:v1:" prefix to restore the prefixless ciphertext shape. | ||
| -- Same regex: only touch strings whose remainder is `iv:tag:ciphertext`. | ||
|
|
||
| UPDATE public.agent_connections AS ac | ||
| SET config = sub.fixed_config | ||
| FROM ( | ||
| SELECT | ||
| id, | ||
| jsonb_object_agg( | ||
| key, | ||
| CASE | ||
| WHEN jsonb_typeof(value) = 'string' | ||
| AND value #>> '{}' LIKE 'enc:v1:%' | ||
| AND substring(value #>> '{}' FROM 8) ~ '^[0-9a-f]{24}:[0-9a-f]{32}:[0-9a-f]+$' | ||
| THEN to_jsonb(substring(value #>> '{}' FROM 8)) | ||
| ELSE value | ||
| END | ||
| ) AS fixed_config | ||
| FROM public.agent_connections, | ||
| LATERAL jsonb_each(config) | ||
| GROUP BY id | ||
| ) AS sub | ||
| WHERE ac.id = sub.id | ||
| AND ac.config IS DISTINCT FROM sub.fixed_config; | ||
97 changes: 97 additions & 0 deletions
97
packages/owletto-backend/src/lobu/stores/__tests__/postgres-stores.test.ts
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,97 @@ | ||
| /** | ||
| * Encrypt/decrypt round-trip tests for the connection-config helpers in | ||
| * postgres-stores.ts. Pins the fix for the prefix asymmetry: encrypt now | ||
| * tags ciphertext with `enc:v1:` and decrypt strips it before delegating | ||
| * to @lobu/core's AES-GCM `decrypt()`. | ||
| */ | ||
|
|
||
| import { describe, expect, it } from 'vitest'; | ||
| import { encrypt } from '@lobu/core'; | ||
| import { decryptConfig, encryptConfig } from '../postgres-stores'; | ||
|
|
||
| describe('postgres-stores connection-config encryption', () => { | ||
| it('round-trips secret fields through encrypt + decrypt', () => { | ||
| const original = { | ||
| platform: 'slack', | ||
| botToken: 'xoxb-real-secret-value', | ||
| signingSecret: 'shhhh', | ||
| allowGroups: true, | ||
| }; | ||
|
|
||
| const encrypted = encryptConfig(original); | ||
|
|
||
| // Secret fields are tagged with the version prefix and no longer match | ||
| // the plaintext. | ||
| expect(typeof encrypted.botToken).toBe('string'); | ||
| expect(encrypted.botToken).not.toBe(original.botToken); | ||
| expect(encrypted.botToken.startsWith('enc:v1:')).toBe(true); | ||
| expect(encrypted.signingSecret.startsWith('enc:v1:')).toBe(true); | ||
|
|
||
| // Non-secret fields are untouched. | ||
| expect(encrypted.platform).toBe('slack'); | ||
| expect(encrypted.allowGroups).toBe(true); | ||
|
|
||
| const decrypted = decryptConfig(encrypted); | ||
|
|
||
| expect(decrypted).toEqual(original); | ||
| }); | ||
|
|
||
| it('skips already-encrypted secret values on a second encryptConfig pass', () => { | ||
| const original = { token: 'plaintext-token' }; | ||
| const once = encryptConfig(original); | ||
| const twice = encryptConfig(once); | ||
|
|
||
| // Idempotent: a second encryption pass leaves the already-prefixed | ||
| // ciphertext alone instead of double-encrypting. | ||
| expect(twice.token).toBe(once.token); | ||
| expect(decryptConfig(twice).token).toBe('plaintext-token'); | ||
| }); | ||
|
|
||
| it('decryptConfig leaves prefixless values untouched (treated as plaintext)', () => { | ||
| // A bare `iv:tag:ciphertext` value (the legacy shape produced by the | ||
| // pre-fix encryptConfig) does NOT start with `enc:v1:`, so decryptConfig | ||
| // returns it as-is. The migration is what re-prefixes those rows; this | ||
| // assertion locks in the runtime contract that any non-prefixed string | ||
| // is treated as opaque plaintext. | ||
| const rawCipher = encrypt('would-be-plaintext'); | ||
| const result = decryptConfig({ token: rawCipher, platform: 'slack' }); | ||
|
|
||
| expect(result.token).toBe(rawCipher); | ||
| expect(result.platform).toBe('slack'); | ||
| }); | ||
|
|
||
| it('decryptConfig returns the original plaintext for prefixed values', () => { | ||
| const ciphertext = encrypt('super-secret'); | ||
| const result = decryptConfig({ token: `enc:v1:${ciphertext}` }); | ||
|
|
||
| expect(result.token).toBe('super-secret'); | ||
| }); | ||
|
|
||
| it('decryptConfig leaves an undecryptable prefixed value alone', () => { | ||
| // Garbage after the prefix shouldn't crash decryptConfig — the inner | ||
| // try/catch swallows the failure and the caller still gets a value | ||
| // back (the original prefixed string), matching the pre-fix contract. | ||
| const result = decryptConfig({ token: 'enc:v1:not-real-ciphertext' }); | ||
| expect(result.token).toBe('enc:v1:not-real-ciphertext'); | ||
| }); | ||
|
|
||
| it('encryptConfig only touches secret-named fields', () => { | ||
| const input = { | ||
| platform: 'telegram', | ||
| // Not a secret-shaped key name — should pass through untouched. | ||
| label: 'team-prod', | ||
| // Secret-shaped names — should be encrypted. | ||
| botToken: 'tg-token', | ||
| apiKey: 'ak', | ||
| authorization: 'Bearer xyz', | ||
| }; | ||
|
|
||
| const encrypted = encryptConfig(input); | ||
|
|
||
| expect(encrypted.platform).toBe('telegram'); | ||
| expect(encrypted.label).toBe('team-prod'); | ||
| expect(encrypted.botToken.startsWith('enc:v1:')).toBe(true); | ||
| expect(encrypted.apiKey.startsWith('enc:v1:')).toBe(true); | ||
| expect(encrypted.authorization.startsWith('enc:v1:')).toBe(true); | ||
| }); | ||
| }); |
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The migration’s detection regex requires at least one hex char in the ciphertext segment (
[0-9a-f]+), but@lobu/coreencryption can legitimately produceiv:tag:for empty-string secrets (seepackages/core/src/utils/encryption.tswhereencrypted.toString("hex")can be empty). In that case, legacy prefixless rows are skipped by this backfill, sodecryptConfigwill still treat them as opaque strings instead of decrypting them after rollout; this leaves a subset of previously encrypted secret fields unfixed.Useful? React with 👍 / 👎.