bus: row 199 per-agent credential store, store references, issue (tmp, fsync, rename, commit, memory), revoke (memory first, store file only), remove, useFor keyed from the session, not-issued and revoked states (#1697)

Design D1, D3a, D4 and section 6. Tests T1, T2 and the store half of T5 use fake tokens in a scratch
root. A wrong owner can't be set without root, so T1 covers modes, symlinks and dirs only.

packages/bus/src/credentials.mjs

@@ -1,5 +1,8 @@
-import { openSync, closeSync, fstatSync, readFileSync, lstatSync, realpathSync, constants } from 'node:fs';
+import {
-import { isAbsolute, relative, resolve } from 'node:path';
+  openSync, closeSync, fstatSync, readFileSync, lstatSync, realpathSync, constants, mkdirSync, writeSync, fsyncSync,
+  renameSync, unlinkSync, rmdirSync, readdirSync,
+} from 'node:fs';
+import { dirname, isAbsolute, join, relative, resolve } from 'node:path';
 import { BusError } from './broker.mjs';
 const deny = (code) => {
   throw new BusError(code);
@@ -13,9 +16,33 @@ const day = (value) =>
   /^\d{4}-\d{2}-\d{2}$/.test(value) &&
   !Number.isNaN(Date.parse(value)) &&
   new Date(value).toISOString().slice(0, 10) === value;
+const TOKEN = /^[-A-Za-z0-9._~]+\n?$/;
+// Row 199 (#1697, design D1, D4): a store dir is the stack instance's user's own and nobody else's.
+const privateDir = (path, { missing = false } = {}) => {
+  let st;
+  try {
+    st = lstatSync(path);
+  } catch (e) {
+    if (missing && e.code === 'ENOENT') return false;
+    deny('credential-file');
+  }
+  if (!st.isDirectory() || st.uid !== process.getuid() || (st.mode & 0o077) !== 0) deny('credential-file');
+  return true;
+};
 export class Credentials {
   #tokens = new Map();
-  constructor({ references = {}, repoRoots = [], dataRoot, env = process.env }) {
+  // Row 199: every reference the bus loaded, so issue, rotate, revoke and status work per entry.
+  #refs = new Map();
+  #revoked = new Set();
+  #storeRoot;
+  #repoRoots;
+  #dataRoot;
+  // storeRoot is <configDir>/secrets/agents (design D1); a reference with store: true must name the
+  // path under it for its own business, agent and service.
+  constructor({ references = {}, repoRoots = [], dataRoot, env = process.env, storeRoot = null }) {
+    this.#storeRoot = storeRoot;
+    this.#repoRoots = repoRoots;
+    this.#dataRoot = dataRoot;
     try {
       for (const [instance, services] of Object.entries(references))
         for (const [service, ref] of Object.entries(services)) {
@@ -26,12 +53,41 @@ export class Credentials {
           if (
             (ref.service !== undefined && ref.service !== service) ||
             Object.keys(ref).some(
-              (k) => !['service', 'file', 'env', service === 'vikunja' ? 'expires' : 'rotateBy'].includes(k),
+              (k) => !['service', 'file', 'env', 'store', service === 'vikunja' ? 'expires' : 'rotateBy'].includes(k),
             ) ||
-            Boolean(ref.file) === Boolean(ref.env)
+            Boolean(ref.file) === Boolean(ref.env) ||
+            (ref.store !== undefined && (ref.store !== true || ref.file !== this.#storePath(instance, service)))
           )
             deny('credential-reference');
+          this.#refs.set(instance + ':' + service, { ref, date });
+          const token = this.#read(ref, env);
+          if (token !== null) this.#tokens.set(instance + ':' + service, { token, date });
+        }
+    } catch (e) {
+      this.close();
+      throw e;
+    }
+  }
+  #storePath(instance, service) {
+    if (!this.#storeRoot) deny('credential-reference');
+    const [business, agent, ...rest] = instance.split('/');
+    if (!business || !agent || rest.length || agent.startsWith('@')) deny('credential-reference');
+    return join(this.#storeRoot, business, agent, service);
+  }
+  // One token, read and checked, or null for a store reference with no file yet (design D3a).
+  #read(ref, env) {
           let token;
+          if (ref.store) {
+            const agentDir = dirname(ref.file);
+            privateDir(this.#storeRoot);
+            if (!privateDir(dirname(agentDir), { missing: true }) || !privateDir(agentDir, { missing: true })) return null;
+            try {
+              lstatSync(ref.file);
+            } catch (e) {
+              if (e.code === 'ENOENT') return null;
+              deny('credential-file');
+            }
+          }
           if (ref.file) {
             if (!isAbsolute(ref.file)) deny('credential-location');
             let resolved, original;
@@ -43,7 +99,7 @@ export class Credentials {
             }
             if (original.isSymbolicLink() || !original.isFile()) deny('credential-file');
             if (
-              [...repoRoots, dataRoot].filter(Boolean).some((root) =>
+              [...this.#repoRoots, this.#dataRoot].filter(Boolean).some((root) =>
                 inside(
                   (() => {
                     try {
@@ -81,16 +137,98 @@ export class Credentials {
             if (!/^[A-Z_][A-Z0-9_]*$/.test(ref.env)) deny('credential-reference');
             token = env[ref.env];
           }
-          if (typeof token !== 'string' || token.length > 16384 || !/^[-A-Za-z0-9._~]+\n?$/.test(token))
+          return Credentials.#format(token);
-            deny('credential-format');
+  }
+  static #format(token) {
+    if (typeof token !== 'string' || token.length > 16384 || !TOKEN.test(token)) deny('credential-format');
     token = token.replace(/\n$/, '');
     if (token.length < 16) deny('credential-format');
-          this.#tokens.set(instance + ':' + service, { token, date });
+    return token;
   }
+  #known(instance, service) {
+    const known = this.#refs.get(instance + ':' + service);
+    if (!known) deny('credential-unknown');
+    return known;
+  }
+  // Row 199 (design section 6): write the token to the agent's store path (tmp, fsync, rename), then
+  // `commit` (the business file's reference becomes store), then memory. The token comes from the
+  // caller's stdin and is never logged. Returns the entry's new status.
+  issue(instance, service, token, date, commit = () => {}) {
+    this.#known(instance, service);
+    if (!day(date)) deny('credential-date');
+    token = Credentials.#format(token);
+    const file = this.#storePath(instance, service);
+    const agentDir = dirname(file);
+    mkdirSync(this.#storeRoot, { recursive: true, mode: 0o700 });
+    privateDir(this.#storeRoot);
+    for (const dir of [dirname(agentDir), agentDir]) {
+      if (!privateDir(dir, { missing: true })) mkdirSync(dir, { mode: 0o700 });
+      privateDir(dir);
+    }
+    const tmp = file + '.tmp';
+    try {
+      unlinkSync(tmp);
     } catch (e) {
-      this.close();
+      if (e.code !== 'ENOENT') deny('credential-file');
-      throw e;
     }
+    let fd;
+    try {
+      fd = openSync(tmp, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | constants.O_NOFOLLOW, 0o600);
+      writeSync(fd, token + '\n');
+      fsyncSync(fd);
+    } finally {
+      if (fd !== undefined) closeSync(fd);
+    }
+    renameSync(tmp, file);
+    const dirFd = openSync(agentDir, constants.O_RDONLY);
+    try {
+      fsyncSync(dirFd);
+    } finally {
+      closeSync(dirFd);
+    }
+    commit();
+    this.#refs.set(instance + ':' + service, { ref: { service, store: true, file, [service === 'vikunja' ? 'expires' : 'rotateBy']: date }, date });
+    this.#tokens.set(instance + ':' + service, { token, date });
+    this.#revoked.delete(instance + ':' + service);
+    return this.status().find((e) => e.instance === instance && e.service === service);
+  }
+  // Memory first, then the store file (design section 6, D12). A file or env reference outside the
+  // store is never unlinked; the caller turns its reference into store, so a restart finds nothing.
+  revoke(instance, service) {
+    const { ref } = this.#known(instance, service);
+    this.#tokens.delete(instance + ':' + service);
+    this.#revoked.add(instance + ':' + service);
+    if (ref.store) {
+      try {
+        unlinkSync(ref.file);
+      } catch (e) {
+        if (e.code !== 'ENOENT') deny('credential-file');
+      }
+    }
+    return { file: Boolean(ref.store) };
+  }
+  // revoke for every service, then the agent's dir. A file other than a token or its tmp refuses.
+  remove(instance) {
+    const services = [...this.#refs.keys()].filter((k) => k.startsWith(instance + ':')).map((k) => k.slice(instance.length + 1));
+    for (const service of services) this.revoke(instance, service);
+    const agentDir = dirname(this.#storePath(instance, 'gitea'));
+    let names;
+    try {
+      names = readdirSync(agentDir);
+    } catch (e) {
+      if (e.code === 'ENOENT') return { services };
+      deny('credential-file');
+    }
+    if (names.some((n) => !/^(gitea|vikunja)(\.tmp)?$/.test(n))) deny('credential-file');
+    for (const n of names) unlinkSync(join(agentDir, n));
+    rmdirSync(agentDir);
+    return { services };
+  }
+  // The agent-facing use (design D5): the key comes only from the broker's verified session.
+  async useFor(session, service, operation) {
+    if (!session || typeof session.business !== 'string' || typeof session.role !== 'string' || session.role.startsWith('@'))
+      deny('credential-not-yours');
+    return this.use(session.business + '/' + session.role, service, operation);
   }
   assertClean(value) {
     let encoded;
@@ -109,14 +247,16 @@ export class Credentials {
     let result;
     try {
       result = await operation(entry.token);
-    } catch {
+    } catch (e) {
+      // Row 199: a host step's own refusal (publish) keeps its code; anything else is the service's.
+      if (e instanceof BusError) throw e;
       deny('service-failed');
     }
     this.assertClean(result);
     return result;
   }
   status() {
-    return [...this.#tokens.entries()].map(([key, { date }]) => {
+    return [...this.#refs.entries()].map(([key, { date }]) => {
       const i = key.lastIndexOf(':'),
         service = key.slice(i + 1),
         left = Date.parse(date + 'T00:00:00Z') - Date.now();
@@ -124,8 +264,11 @@ export class Credentials {
         instance: key.slice(0, i),
         service,
         date,
-        state:
+        state: !this.#tokens.has(key)
-          service === 'gitea'
+          ? this.#revoked.has(key)
+            ? 'revoked'
+            : 'not-issued'
+          : service === 'gitea'
             ? left <= 0
               ? 'rotation-due'
               : 'valid'
@@ -139,5 +282,6 @@ export class Credentials {
   }
   close() {
     this.#tokens.clear();
+    this.#refs.clear();
   }
 }

packages/bus/tests/credential-store.test.mjs

@@ -0,0 +1,155 @@
+// Row 199 (#1697, design D1, D3a, D4, section 6, T1 and T5): the per-agent credential store under
+// <configDir>/secrets/agents/<business>/<agent>/<service>. Fake tokens in a scratch root only.
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import { Credentials } from '../src/credentials.mjs';
+
+const TOKEN = 'fake_store_token_0123456789';
+const ROTATED = 'fake_rotated_token_9876543210';
+const AGENT = { business: 'acme', role: 'coder' };
+
+function setup(t) {
+  const root = mkdtempSync(join(tmpdir(), 'bus-store-'));
+  t.after(() => rmSync(root, { recursive: true, force: true }));
+  const storeRoot = join(root, 'secrets', 'agents');
+  mkdirSync(storeRoot, { recursive: true, mode: 0o700 });
+  chmodSync(join(root, 'secrets'), 0o700);
+  const file = join(storeRoot, 'acme', 'coder', 'gitea');
+  const references = { 'acme/coder': { gitea: { service: 'gitea', store: true, file, rotateBy: '2099-01-01' } } };
+  const open = (extra = {}) => new Credentials({ references, storeRoot, repoRoots: [], dataRoot: '/nonexistent-data', ...extra });
+  const state = (c) => c.status().find((e) => e.instance === 'acme/coder' && e.service === 'gitea').state;
+  const read = (c, session = AGENT) => c.useFor(session, 'gitea', async (token) => ({ same: token === TOKEN, rotated: token === ROTATED }));
+  return { root, storeRoot, file, references, open, state, read };
+}
+
+test('T1: a store reference with no file yet starts, and the agent is not-issued until issue', async (t) => {
+  const s = setup(t);
+  const c = s.open();
+  assert.equal(s.state(c), 'not-issued');
+  await assert.rejects(s.read(c), /credential-unavailable/);
+  const status = c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01');
+  assert.equal(status.state, 'valid');
+  assert.deepEqual(await s.read(c), { same: true, rotated: false });
+  assert.equal(statSync(s.file).mode & 0o777, 0o600);
+  for (const dir of [join(s.storeRoot, 'acme'), join(s.storeRoot, 'acme', 'coder')]) assert.equal(statSync(dir).mode & 0o777, 0o700);
+  assert.deepEqual(readdirSync(join(s.storeRoot, 'acme', 'coder')), ['gitea']);
+  // A restart reads the issued file.
+  assert.deepEqual(await s.read(s.open()), { same: true, rotated: false });
+});
+
+test('T1: a store path the reference does not own, or no store root, refuses the reference', (t) => {
+  const s = setup(t);
+  const other = join(s.storeRoot, 'acme', 'pm', 'gitea');
+  assert.throws(() => s.open({ references: { 'acme/coder': { gitea: { store: true, file: other, rotateBy: '2099-01-01' } } } }), /credential-reference/);
+  assert.throws(() => new Credentials({ references: s.references, repoRoots: [], dataRoot: '/nonexistent-data' }), /credential-reference/);
+  assert.throws(() => s.open({ references: { 'acme/@tester': { gitea: { store: true, file: join(s.storeRoot, 'acme', '@tester', 'gitea'), rotateBy: '2099-01-01' } } } }),
+    /credential-reference/);
+});
+
+test('T1: a wrong mode on the store root, a business dir, an agent dir or the file refuses start', (t) => {
+  const s = setup(t);
+  s.open().issue('acme/coder', 'gitea', TOKEN, '2099-01-01');
+  for (const [path, bad, good] of [
+    [s.storeRoot, 0o750, 0o700],
+    [join(s.storeRoot, 'acme'), 0o705, 0o700],
+    [join(s.storeRoot, 'acme', 'coder'), 0o770, 0o700],
+    [s.file, 0o640, 0o600],
+  ]) {
+    chmodSync(path, bad);
+    assert.throws(() => s.open(), /credential-file/, path);
+    chmodSync(path, good);
+  }
+  s.open();
+});
+
+test('T1: a missing store root refuses start; a missing {file} reference still refuses start', (t) => {
+  const s = setup(t);
+  rmSync(s.storeRoot, { recursive: true });
+  assert.throws(() => s.open(), /credential-file/);
+  assert.throws(() => s.open({ references: { coder: { gitea: { file: join(s.root, 'absent'), rotateBy: '2099-01-01' } } } }), /credential-file/);
+});
+
+test('T1: a store file that is a directory or a symlink refuses start', (t) => {
+  const s = setup(t);
+  mkdirSync(join(s.storeRoot, 'acme', 'coder'), { recursive: true, mode: 0o700 });
+  chmodSync(join(s.storeRoot, 'acme'), 0o700);
+  mkdirSync(s.file, { mode: 0o700 });
+  assert.throws(() => s.open(), /credential-file/);
+  rmSync(s.file, { recursive: true });
+  const target = join(s.root, 'target');
+  writeFileSync(target, TOKEN + '\n', { mode: 0o600 });
+  symlinkSync(target, s.file);
+  assert.throws(() => s.open(), /credential-file/);
+});
+
+test('T2: useFor keys from the session only', async (t) => {
+  const s = setup(t);
+  const c = s.open();
+  c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01');
+  await assert.rejects(s.read(c, { business: 'acme', role: 'pm' }), /credential-unavailable/);
+  await assert.rejects(s.read(c, { business: 'acme', role: '@tester' }), /credential-not-yours/);
+  await assert.rejects(s.read(c, null), /credential-not-yours/);
+  await assert.rejects(s.read(c, { business: 'acme' }), /credential-not-yours/);
+});
+
+test('T5: issue, rotate, revoke and delete are live with no restart, and revoke holds across one', async (t) => {
+  const s = setup(t);
+  const c = s.open();
+  let committed = 0;
+  c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01', () => committed++);
+  assert.equal(committed, 1);
+  // Rotate is a second issue: the new token is used at once, with no tmp file left.
+  c.issue('acme/coder', 'gitea', ROTATED + '\n', '2099-06-01');
+  assert.deepEqual(await s.read(c), { same: false, rotated: true });
+  assert.equal(c.status()[0].date, '2099-06-01');
+  assert.deepEqual(readdirSync(join(s.storeRoot, 'acme', 'coder')), ['gitea']);
+  // Revoke: memory first, then the file.
+  assert.deepEqual(c.revoke('acme/coder', 'gitea'), { file: true });
+  assert.equal(s.state(c), 'revoked');
+  assert.equal(existsSync(s.file), false);
+  await assert.rejects(s.read(c), /credential-unavailable/);
+  const restarted = s.open();
+  await assert.rejects(s.read(restarted), /credential-unavailable/);
+  assert.equal(s.state(restarted), 'not-issued');
+  // Delete removes the agent's dir.
+  c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01');
+  assert.deepEqual(c.remove('acme/coder'), { services: ['gitea'] });
+  assert.equal(existsSync(join(s.storeRoot, 'acme', 'coder')), false);
+  await assert.rejects(s.read(c), /credential-unavailable/);
+});
+
+test('T5: a failed commit leaves the token on disk but not in memory, and a bad token or date writes nothing', async (t) => {
+  const s = setup(t);
+  const c = s.open();
+  assert.throws(() => c.issue('acme/coder', 'gitea', 'short', '2099-01-01'), /credential-format/);
+  assert.throws(() => c.issue('acme/coder', 'gitea', TOKEN, '2099-13-01'), /credential-date/);
+  assert.throws(() => c.issue('acme/pm', 'gitea', TOKEN, '2099-01-01'), /credential-unknown/);
+  assert.equal(existsSync(s.file), false);
+  assert.throws(() => c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01', () => { throw new Error('commit failed'); }), /commit failed/);
+  assert.equal(existsSync(s.file), true);
+  assert.equal(s.state(c), 'not-issued');
+  await assert.rejects(s.read(c), /credential-unavailable/);
+});
+
+test('T5: revoke of a {file} reference forgets the token and never unlinks the file', async (t) => {
+  const s = setup(t);
+  const path = join(s.root, 'outside-token');
+  writeFileSync(path, TOKEN + '\n', { mode: 0o600 });
+  const c = s.open({ references: { 'acme/coder': { gitea: { file: path, rotateBy: '2099-01-01' } } } });
+  assert.deepEqual(await s.read(c), { same: true, rotated: false });
+  assert.deepEqual(c.revoke('acme/coder', 'gitea'), { file: false });
+  assert.equal(existsSync(path), true);
+  await assert.rejects(s.read(c), /credential-unavailable/);
+});
+
+test('T5: delete refuses an agent dir holding anything but tokens', (t) => {
+  const s = setup(t);
+  const c = s.open();
+  c.issue('acme/coder', 'gitea', TOKEN, '2099-01-01');
+  writeFileSync(join(s.storeRoot, 'acme', 'coder', 'notes'), 'x', { mode: 0o600 });
+  assert.throws(() => c.remove('acme/coder'), /credential-file/);
+  assert.equal(existsSync(join(s.storeRoot, 'acme', 'coder', 'notes')), true);
+});