-
Notifications
You must be signed in to change notification settings - Fork 0
feat(security): encrypt NotificationChannel.config secrets at rest #198
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
Changes from 8 commits
10a7725
b280005
c43abad
c4e6281
0369617
c1ab046
5b20c4f
9562694
5702fa0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,28 @@ | ||
| #!/usr/bin/env tsx | ||
| import { prisma } from "@/lib/prisma"; | ||
| import { encryptChannelConfig, SENSITIVE_FIELDS_BY_TYPE } from "@/server/services/channel-secrets"; | ||
|
|
||
| async function main() { | ||
| const channels = await prisma.notificationChannel.findMany({ | ||
| select: { id: true, type: true, config: true }, | ||
| }); | ||
|
|
||
| let updated = 0, skipped = 0; | ||
| for (const ch of channels) { | ||
| if (!SENSITIVE_FIELDS_BY_TYPE[ch.type]) { skipped++; continue; } | ||
| const config = ch.config as Record<string, unknown>; | ||
| const encrypted = encryptChannelConfig(ch.type, config); | ||
| // Only write if at least one field actually changed (idempotent skip) | ||
| const changed = Object.keys(encrypted).some((k) => encrypted[k] !== config[k]); | ||
| if (!changed) { skipped++; continue; } | ||
| await prisma.notificationChannel.update({ | ||
| where: { id: ch.id }, | ||
| data: { config: encrypted as never }, | ||
| }); | ||
| updated++; | ||
| console.log(`encrypted channel ${ch.id} (${ch.type})`); | ||
| } | ||
| console.log(`done — updated=${updated} skipped=${skipped} total=${channels.length}`); | ||
| } | ||
|
|
||
| main().catch((e) => { console.error(e); process.exit(1); }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,52 @@ | ||
| import { describe, it, expect, beforeAll } from "vitest"; | ||
| import { encryptChannelConfig, decryptChannelConfig } from "../channel-secrets"; | ||
|
|
||
| beforeAll(() => { | ||
| process.env.NEXTAUTH_SECRET = "test-secret-for-vitest-runs-only"; | ||
| }); | ||
|
|
||
| describe("encryptChannelConfig", () => { | ||
| it("encrypts webhook hmacSecret as v2:-prefixed ciphertext", () => { | ||
| const out = encryptChannelConfig("webhook", { | ||
| url: "https://example.com/hook", | ||
| hmacSecret: "raw-hmac-secret", | ||
| }); | ||
| expect(out.url).toBe("https://example.com/hook"); | ||
| expect(typeof out.hmacSecret).toBe("string"); | ||
| expect(out.hmacSecret as string).toMatch(/^v2:/); | ||
| }); | ||
| }); | ||
|
|
||
| describe("encrypt+decrypt round trip", () => { | ||
| it.each([ | ||
| ["webhook", { url: "https://x.test/h", hmacSecret: "s1", headers: { Authorization: "Bearer abc" } }], | ||
| ["slack", { webhookUrl: "https://hooks.slack.com/services/T/B/XYZ" }], | ||
| ["pagerduty", { integrationKey: "ROUTING-KEY-123", severityMap: { error: "warning" } }], | ||
| ["email", { smtpHost: "smtp.x", smtpUser: "u", smtpPass: "p", recipients: ["a@b"] }], | ||
| ])("round-trips %s", (type, plain) => { | ||
| const enc = encryptChannelConfig(type, plain); | ||
| const dec = decryptChannelConfig(type, enc); | ||
| expect(dec).toEqual(plain); | ||
| }); | ||
| }); | ||
|
|
||
| describe("idempotency", () => { | ||
| it("does not re-encrypt already v2-prefixed values", () => { | ||
| const once = encryptChannelConfig("webhook", { hmacSecret: "raw" }); | ||
| const twice = encryptChannelConfig("webhook", once); | ||
| expect(twice.hmacSecret).toBe(once.hmacSecret); | ||
| }); | ||
|
|
||
| it("decrypt is no-op on plaintext", () => { | ||
| const out = decryptChannelConfig("webhook", { hmacSecret: "plain" }); | ||
| expect(out.hmacSecret).toBe("plain"); | ||
| }); | ||
| }); | ||
|
|
||
| describe("unknown type", () => { | ||
| it("returns config unchanged for unknown channel type", () => { | ||
| const cfg = { foo: "bar" }; | ||
| expect(encryptChannelConfig("unknown", cfg)).toEqual(cfg); | ||
| expect(decryptChannelConfig("unknown", cfg)).toEqual(cfg); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,57 @@ | ||
| import { encrypt, decrypt, ENCRYPTION_DOMAINS } from "./crypto"; | ||
|
|
||
| export const SENSITIVE_FIELDS_BY_TYPE: Record<string, readonly string[]> = { | ||
| webhook: ["hmacSecret", "headers"], | ||
| slack: ["webhookUrl"], | ||
| pagerduty: ["integrationKey"], | ||
| email: ["smtpPass"], | ||
| }; | ||
|
|
||
| const V2_PREFIX = "v2:"; | ||
|
|
||
| function isAlreadyEncrypted(value: unknown): boolean { | ||
| return typeof value === "string" && value.startsWith(V2_PREFIX); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The Useful? React with 👍 / 👎. |
||
| } | ||
|
|
||
| function serializeForEncryption(value: unknown): string { | ||
| return typeof value === "string" ? value : JSON.stringify(value); | ||
| } | ||
|
|
||
| export function encryptChannelConfig( | ||
| type: string, | ||
| config: Record<string, unknown>, | ||
| ): Record<string, unknown> { | ||
| const fields = SENSITIVE_FIELDS_BY_TYPE[type]; | ||
| if (!fields) return config; | ||
|
|
||
| const out: Record<string, unknown> = { ...config }; | ||
| for (const field of fields) { | ||
| const value = out[field]; | ||
| if (value === undefined || value === null || value === "") continue; | ||
| if (isAlreadyEncrypted(value)) continue; | ||
| out[field] = encrypt(serializeForEncryption(value), ENCRYPTION_DOMAINS.SECRETS); | ||
| } | ||
| return out; | ||
| } | ||
|
|
||
| export function decryptChannelConfig( | ||
| type: string, | ||
| config: Record<string, unknown>, | ||
| ): Record<string, unknown> { | ||
| const fields = SENSITIVE_FIELDS_BY_TYPE[type]; | ||
| if (!fields) return config; | ||
|
|
||
| const out: Record<string, unknown> = { ...config }; | ||
| for (const field of fields) { | ||
| const value = out[field]; | ||
| if (typeof value !== "string" || !value.startsWith(V2_PREFIX)) continue; | ||
| const plaintext = decrypt(value, ENCRYPTION_DOMAINS.SECRETS); | ||
| // headers was originally an object; restore it | ||
| if (field === "headers") { | ||
| out[field] = JSON.parse(plaintext); | ||
| } else { | ||
| out[field] = plaintext; | ||
| } | ||
| } | ||
| return out; | ||
| } | ||
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.
Adding
webhook.headersandslack.webhookUrlto the encrypted field map causes those values to be stored asv2:ciphertext, butlistChannelsstill returns channel config values as-is for those keys. The UI edit flow reads them directly (formFromConfig) and then re-validates as URL/JSON-object on submit, so edited Slack channels and webhook channels with headers can fail updates unless users manually re-enter those values. This introduces a regression in the channel-edit workflow after encryption is enabled.Useful? React with 👍 / 👎.