secret store holds foreign secrets, and notify reads from it

putSecret/getSecret, `bun run secret:set`, and notify taking its credentials
from the store with env as the fallback. this was described two messages ago
as a later thing; it is about fifteen lines and it was blocking the honest
answer to "where does the .p8 go", so it is done.

getKey was the wrong door for anything supplied from outside — it MINTS a
random 32-byte value when the purpose is absent, which for a foreign
credential means silently authenticating to apple with a fake. getSecret
returns null instead, which is what lets the env fallback work.

putSecret retires the previous row rather than deleting it: a mistyped paste
over a working key should be recoverable, and retiredKeys already exists for
that shape. one transaction, because the partial unique index means the retire
has to land before the insert.

the CLI reads stdin and has deliberately no --value flag: a secret in argv is
in shell history, in ps output, and in any audit log that records command
lines. offering the flag would make the convenient path the leaky one. it
prints a length and the last four characters, never the value.

also `bun run notify:test`, which fires officer/core:test at a registered
device. the sidecar's port is ephemeral, so it now writes its address to
data/notify-server.url on boot and removes it on clean shutdown; the CLI
treats an unreachable address as "not running" rather than trusting the file,
because a crash leaves one behind.

the script says what actually happened, because the day-waster here is that a
send with nothing configured returns success and delivers nothing — it refuses
up front when channels is empty, and calls out sent:0/failed:0 as "no device
registered" rather than letting it read as success.

verified end to end with a p-256 key generated locally and then deleted from
the store: zero APNS vars in .env, sidecar reported channels: apns, and the
send reached the channel. store is back to jwt and offscale only.
This commit is contained in:
2026-08-19 19:57:51 +00:00
parent 2cefa3d378
commit 16369f3570
10 changed files with 288 additions and 27 deletions
@@ -144,6 +144,49 @@ export function getKey(purpose: string): string {
}
}
/**
* Read a secret WITHOUT creating one. Null when the purpose has never been set.
*
* `getKey` is the wrong door for anything supplied from outside: it mints a random 32-byte value when the
* purpose is absent, which for a foreign credential means silently substituting a key that authenticates
* to nothing. A caller that has an env fallback needs to be told "not here", not handed a fake.
*/
export function getSecret(purpose: string): string | null {
return readActive(purpose);
}
/**
* Store a secret the platform did NOT generate — an APNs `.p8`, an FCM service account, a webhook URL.
*
* Everything else in this store is minted here, which is why `getKey` creates on demand. These arrive
* from outside and can never be regenerated, so they are written rather than derived, and replacing one
* RETIRES the old row instead of deleting it: a mistyped paste that overwrites a working key should be
* recoverable, and `retiredKeys` already exists for exactly this shape.
*
* Returns the previous value, or null if there was none — so a caller can tell "set" from "replaced"
* without reading first and racing itself.
*/
export function putSecret(purpose: string, value: string): string | null {
if (!value) throw new Error('secret-store: refusing to store an empty secret');
const handle = open();
const now = Math.floor(Date.now() / 1000);
// One transaction, because the partial unique index means the retire MUST land before the insert —
// two active rows for one purpose is the state it exists to prevent.
const previous = handle.transaction((): string | null => {
const existing = readActive(purpose);
if (existing === value) return existing; // idempotent: re-running a setup script is not a rotation
if (existing !== null) {
handle.query('UPDATE keys SET retired_at = ? WHERE purpose = ? AND retired_at IS NULL').run(now, purpose);
}
handle.query('INSERT INTO keys (purpose, key, created_at) VALUES (?, ?, ?)').run(purpose, value, now);
return existing;
})();
return previous;
}
/** Whether a purpose has an active key, without creating one. */
export function hasKey(purpose: string): boolean {
return readActive(purpose) !== null;
+26 -3
View File
@@ -1,4 +1,5 @@
import http2 from 'node:http2';
import { getSecret } from 'officerdb/secret-store';
import { getPushDevices, deletePushDevice, recordPushFailure, markPushDeviceSeen } from 'officerdb';
import type { Channel, DeliveryResult, Notification, SendContext } from './types';
import { ApnsAuthToken } from './apns-jwt';
@@ -22,10 +23,32 @@ const HOSTS = {
/** Apple's reasons for "this token is dead" — the row goes immediately rather than after three strikes. */
const DEAD_TOKEN_REASONS = new Set(['BadDeviceToken', 'Unregistered', 'DeviceTokenNotForTopic']);
// `OFFICERDEV_X ?? X` — the house convention since the env prefixing, so an install that predates it
// keeps working. See src/servers/env-prefix.ts.
// Secret store first, env second.
//
// The store is where these belong: bun auto-loads `.env` into every PM2 process, so a key there is
// readable from `/proc/<pid>/environ` of twenty of them, and `officer-music` has no business being able
// to read a credential that can push to the owner's apps forever and never expires.
//
// `getSecret`, NOT `getKey` — `getKey` mints a random value when the purpose is absent, which for a
// credential supplied from outside would mean silently authenticating to Apple with a fake. Null is the
// answer that lets the env fallback work.
//
// Env stays as the fallback so an install that predates the store, or one mid-migration, keeps working.
// `OFFICERDEV_X ?? X` is the house convention — see src/servers/env-prefix.ts.
const env = (name: string) => process.env[`OFFICERDEV_${name}`] ?? process.env[name];
const [APNS_KEY_P8, APNS_KEY_ID, APNS_TEAM_ID] = [env('APNS_KEY_P8'), env('APNS_KEY_ID'), env('APNS_TEAM_ID')];
const stored = (purpose: string): string | undefined => {
try {
return getSecret(purpose) ?? undefined;
} catch {
// A store that cannot be opened must not take the sidecar down: the env fallback may well hold the
// key, and a notifier that refuses to boot is worse than one with a channel switched off.
return undefined;
}
};
const APNS_KEY_P8 = stored('apns-p8') ?? env('APNS_KEY_P8');
const APNS_KEY_ID = stored('apns-key-id') ?? env('APNS_KEY_ID');
const APNS_TEAM_ID = stored('apns-team-id') ?? env('APNS_TEAM_ID');
const auth =
APNS_KEY_P8 && APNS_KEY_ID && APNS_TEAM_ID
+11 -1
View File
@@ -1,3 +1,4 @@
import { getSecret } from 'officerdb/secret-store';
import type { Channel, Notification, SendContext } from './types';
import { renderNotification } from './render';
@@ -15,7 +16,16 @@ import { renderNotification } from './render';
// and its history is searchable and permanent in a way a push banner is not — arguably it deserves the
// rule more, not less.
const WEBHOOK_URL = process.env.OFFICERDEV_DISCORD_WEBHOOK_URL ?? process.env.DISCORD_WEBHOOK_URL;
// Store first, env second — same reasoning as apns.ts.
const WEBHOOK_URL = (() => {
try {
const stored = getSecret('discord-webhook');
if (stored) return stored;
} catch {
/* unreadable store — fall through to env */
}
return process.env.OFFICERDEV_DISCORD_WEBHOOK_URL ?? process.env.DISCORD_WEBHOOK_URL;
})();
export const discordChannel: Channel = {
name: 'discord',
+11 -1
View File
@@ -1,3 +1,4 @@
import { getSecret } from 'officerdb/secret-store';
import { getPushDevices, deletePushDevice, recordPushFailure, markPushDeviceSeen } from 'officerdb';
import type { Channel, DeliveryResult, Notification, SendContext } from './types';
import { FcmAccessToken, parseServiceAccount } from './fcm-auth';
@@ -17,7 +18,16 @@ const ENDPOINT = (projectId: string) => `https://fcm.googleapis.com/v1/projects/
/** Google's ways of saying "this token is dead" — the row goes immediately. */
const DEAD_TOKEN_CODES = new Set(['UNREGISTERED', 'INVALID_ARGUMENT', 'NOT_FOUND']);
const raw = process.env.OFFICERDEV_FCM_SERVICE_ACCOUNT ?? process.env.FCM_SERVICE_ACCOUNT;
// Store first, env second — same reasoning as apns.ts.
const raw = (() => {
try {
const stored = getSecret('fcm-service-account');
if (stored) return stored;
} catch {
/* unreadable store — fall through to env */
}
return process.env.OFFICERDEV_FCM_SERVICE_ACCOUNT ?? process.env.FCM_SERVICE_ACCOUNT;
})();
const auth = raw ? new FcmAccessToken(parseServiceAccount(raw)) : null;
type FcmMessage = {
+15
View File
@@ -7,6 +7,9 @@ import { resolveNotifyUser } from './resolve-user';
import { closeApnsSessions } from './apns';
import type { Notification } from './types';
import { API_URL } from '../../officer-url.mjs';
import { DATA_PATH } from '../../data-path';
import { writeFileSync, rmSync } from 'node:fs';
import { join } from 'node:path';
// The officer-notify sidecar. The one place anything leaves this machine to tell the owner something.
//
@@ -51,6 +54,13 @@ function getFreePort(): number {
const port = getFreePort();
// Where this sidecar can be reached, written down for processes that are not the platform.
//
// The port is ephemeral by design, and the platform learns it through sidecar registration — but a CLI
// (`bun run notify:test`) has no session and no registry, and scraping `pm2 logs` for it is the kind of
// thing that works until it doesn't. One file, rewritten on every boot, deleted on clean shutdown.
const ADDRESS_FILE = join(DATA_PATH, 'notify-server.url');
const server = Bun.serve({
port,
hostname: '127.0.0.1',
@@ -112,6 +122,8 @@ const server = Bun.serve({
},
});
writeFileSync(ADDRESS_FILE, `http://127.0.0.1:${server.port}\n`);
console.log(
`[notify] listening on http://127.0.0.1:${server.port} — channels: ${configuredChannels().join(', ') || 'none configured'}`,
);
@@ -140,6 +152,9 @@ const connection = createSidecarConnector({
function shutdown(signal: string) {
console.log(`[notify] ${signal} received, shutting down...`);
// So a stale address cannot outlive the listener. A crash still leaves one behind, which is why the
// CLI treats an unreachable address as "not running" rather than trusting the file.
rmSync(ADDRESS_FILE, { force: true });
closeApnsSessions();
connection.destroy();
process.exit(0);