Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
79 changes: 79 additions & 0 deletions middleware/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,14 @@ import type {
import { createMemoryRouter } from './routes/memory.js';
import { createDatasetsRouter } from './routes/datasets.js';
import { createBulkPromotionRouter } from './routes/bulkPromotion.js';
import { createSkillPromotionRouter } from './routes/skillPromotion.js';
import { PgSkillOwnershipLifecycleStore } from './services/skillLifecycleStore.js';
import { resolveSkillManifestSigningKey } from './services/skillManifestSigningKey.js';
import { createCredentialAskRouter } from './routes/credentialAsks.js';
import { InMemoryCredentialAskStore } from './credentials/asks.js';
import { PostgresCredentialAskStore } from './credentials/postgresCredentialAskStore.js';
import { resolveCredentialMasterKey } from './credentials/crypto.js';
import { createCredentialStore } from './credentials/credentialStoreFactory.js';
import { createInconsistenciesRouter } from './routes/inconsistencies.js';
import { createDuplicatesRouter } from './routes/duplicates.js';
import { createTopicsRouter } from './routes/topics.js';
Expand Down Expand Up @@ -803,6 +811,30 @@ async function main(): Promise<void> {
// runtimes are constructed) because it doubles as the key the `ctx.flows`
// toolkit signs plugin-flow state with (spec 004 FR-B3).
const sessionSigningKey = await resolveSessionSigningKey(secretVault);
// #778 W1 — HMAC key `promoteSkillOwnerScope` (#577 P3) re-signs a skill's
// manifest with. Resolved here alongside the session key: same vault,
// same "generate once, persist, reuse every boot" pattern — see
// `services/skillManifestSigningKey.ts`.
const skillManifestSigningKey = await resolveSkillManifestSigningKey(secretVault);
// #778 W1 — the credential keychain's own master key (#578 Phase 1),
// resolved but never actually used anywhere until now. Same
// `resolveMasterKey` call `credentials/crypto.ts`'s module doc documents
// (`CREDENTIAL_KEYCHAIN_KEY` env, deliberately a DIFFERENT key/env var than
// `VAULT_KEY` — different trust domain). Needed here because
// `InMemoryCredentialAskStore` (the no-Postgres fallback) holds a live
// `CredentialStore` reference to validate an ask's `credentialId` in
// process, the same way `PostgresCredentialAskStore` validates it via SQL.
const credentialMasterKey = await resolveCredentialMasterKey(
DATA_DIR,
process.env['NODE_ENV'] === 'production',
);
if (credentialMasterKey.source === 'env') {
console.log('[middleware] credential-keychain master key loaded from CREDENTIAL_KEYCHAIN_KEY env');
} else if (credentialMasterKey.source === 'dev-file-existed') {
console.log('[middleware] ⚠ credential-keychain master key loaded from dev file — set CREDENTIAL_KEYCHAIN_KEY for production');
} else {
console.warn('[middleware] ⚠ credential-keychain master key GENERATED (dev file) — DEV ONLY. Set CREDENTIAL_KEYCHAIN_KEY for production.');
}
// Spec 004 (FR-B5) — origin plugin flow callbacks resolve against.
const flowPublicBaseUrl =
config.FLOW_PUBLIC_BASE_URL ?? config.PUBLIC_BASE_URL;
Expand Down Expand Up @@ -2952,6 +2984,53 @@ async function main(): Promise<void> {
);
}

// #778 W1 — #577 P3's admin-gated skill promotion route. Deliberately
// deferred by #771 to keep that PR's blast radius to new files only (see
// its "Not in this PR" section) — this is the mount. `PgSkillOwnershipLifecycleStore`
// needs a real Postgres pool (raw SQL over the `skills` table's #577
// columns), so it is only constructed/mounted when `graphPool` is
// available, the same gate `bulkPromotionService` above uses. `requireAuth`
// gates the router; the router's own `requireSessionUserId` check replicates
// the `routes/bulkPromotion.ts` auth chain exactly (single-tenant byte5 —
// every authenticated session is an operator).
if (graphPool) {
const skillLifecycleStore = new PgSkillOwnershipLifecycleStore(graphPool);
app.use(
'/api/v1/admin/skills',
requireAuth,
createSkillPromotionRouter({ store: skillLifecycleStore, signingKey: skillManifestSigningKey }),
);
console.log(
'[middleware] skill-promotion endpoint ready at /api/v1/admin/skills/:skillId/promote',
);
} else {
console.log(
'[middleware] skill-promotion endpoint skipped — no graphPool (Neon backend missing?)',
);
}

// #778 W1 — #578 Phase 3's keychain-asks HTTP surface. Built and
// route-tested by #774 but deliberately left unmounted (same "new files
// only" blast-radius discipline as #577 P3) — this is the mount.
// `CredentialAskStore` follows the exact backend-choice precedent
// `credentials/credentialStoreFactory.ts` documents for the credential
// keychain itself: Postgres when a pool is configured, in-memory
// otherwise (works within one process; asks do not survive a restart).
// `requireAuth` gates the router, per that file's own module doc
// ("behind `requireAuth` like every other `/api/v1/admin/*` router").
const { store: credentialStoreForAsks } = createCredentialStore(graphPool, credentialMasterKey.key);
const credentialAskStore = graphPool
? new PostgresCredentialAskStore(graphPool)
: new InMemoryCredentialAskStore(credentialStoreForAsks);
app.use(
'/api/v1/admin/credential-asks',
requireAuth,
createCredentialAskRouter({ store: credentialAskStore }),
);
console.log(
`[middleware] credential-asks endpoint ready at /api/v1/admin/credential-asks (backend=${graphPool ? 'postgres' : 'in-memory'})`,
);

// Slice 9 — inconsistency detection workflow. Always mount (the
// routes work without a detector — manual /detect 503s, list/get/
// resolve work because they only touch the KG). Resolve hits the
Expand Down
125 changes: 125 additions & 0 deletions middleware/src/routes/skillPromotion.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
import { Router } from 'express';
import type { Request, Response } from 'express';
import { z } from 'zod';

import {
SkillLifecycleTransitionRejected,
type PgSkillOwnershipLifecycleStore,
type SkillOwnershipLifecycleRow,
} from '../services/skillLifecycleStore.js';
import { SkillAutomationWriteBlocked } from '../services/skillLifecycle.js';

/**
* #778 W1 — REST surface for `PgSkillOwnershipLifecycleStore.promoteSkillOwnerScope`
* (#577 P3), the only path a skill ever reaches `group`/`org` ownership.
* Mounted under `/api/v1/admin/skills`.
*
* One endpoint:
* POST /:skillId/promote → promote an already-published skill to a
* team (group) or org home, re-signing its
* manifest at the new owner scope.
*
* Auth follows the EXACT `routes/bulkPromotion.ts` precedent
* (`req.session.omadia_user_id`, single-tenant byte5 — every authenticated
* session is an operator). `promoteSkillOwnerScope` itself has no notion of
* roles; this route's session check IS the "admin-gated" half #577 P3's PR
* description explicitly left to the route layer. A subtly wrong auth check
* here is a security regression — this is why the route was not rushed
* alongside the service layer.
*/

const TargetScopeSchema = z.discriminatedUnion('kind', [
z.object({ kind: z.literal('group'), groupRef: z.string().min(1) }),
z.object({ kind: z.literal('org'), orgId: z.string().min(1) }),
]);

const PromoteBodySchema = z.object({
targetScope: TargetScopeSchema,
});

function requireSessionUserId(req: Request, res: Response): string | null {
const id = req.session?.omadia_user_id;
if (!id) {
res.status(401).json({ code: 'auth.required', message: 'login required' });
return null;
}
return id;
}

function toSkillBody(row: SkillOwnershipLifecycleRow) {
return {
id: row.id,
slug: row.slug,
name: row.name,
ownerScope: row.ownerScope,
lifecycleStatus: row.lifecycleStatus,
manifestSignedAt: row.manifestSignedAt ? row.manifestSignedAt.toISOString() : null,
};
}

function errMsg(err: unknown): string {
return err instanceof Error ? err.message : String(err);
}

export interface SkillPromotionRouteDeps {
/** Narrowed to the one method this route calls (`Pick`, not the concrete
* class) so a test double can stand in without a real `Pool`. */
readonly store: Pick<PgSkillOwnershipLifecycleStore, 'promoteSkillOwnerScope'>;
/** HMAC key `promoteSkillOwnerScope` re-signs the manifest with — see
* `services/skillManifestSigningKey.ts`. */
readonly signingKey: string;
}

export function createSkillPromotionRouter(deps: SkillPromotionRouteDeps): Router {
const router = Router();

router.post('/:skillId/promote', async (req: Request, res: Response): Promise<void> => {
const sessionUserId = requireSessionUserId(req, res);
if (!sessionUserId) return;

const skillId = req.params.skillId as string;
const parsed = PromoteBodySchema.safeParse(req.body ?? {});
if (!parsed.success) {
res.status(400).json({ code: 'skill_promotion.invalid_request', issues: parsed.error.issues });
return;
}

try {
const updated = await deps.store.promoteSkillOwnerScope(skillId, parsed.data.targetScope, {
actorScope: { kind: 'personal', userId: sessionUserId },
signingKey: deps.signingKey,
});
res.json(toSkillBody(updated));
} catch (err) {
if (err instanceof SkillAutomationWriteBlocked) {
// Unreachable today (actorScope is always 'personal' here, never
// 'system') but handled explicitly rather than falling into the
// generic 500 branch below — a machine actor being rejected is a
// 403, not a server error.
res.status(403).json({ code: 'skill_promotion.automation_blocked', message: err.message });
return;
}
if (err instanceof SkillLifecycleTransitionRejected) {
res.status(409).json({
code: 'skill_promotion.transition_rejected',
reason: err.reason,
...(err.missing ? { missing: err.missing } : {}),
message: err.message,
});
return;
}
const message = errMsg(err);
if (message.includes('not found')) {
res.status(404).json({ code: 'skill_promotion.not_found', message });
return;
}
if (message.includes('is not published') || message.includes('has no owner scope yet')) {
res.status(409).json({ code: 'skill_promotion.not_eligible', message });
return;
}
res.status(500).json({ code: 'skill_promotion.failed', message });
}
});

return router;
}
35 changes: 35 additions & 0 deletions middleware/src/services/skillManifestSigningKey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import crypto from 'node:crypto';

import type { SecretVault } from '../secrets/vault.js';

/**
* #778 W1 — the HMAC key `signSkillManifest`/`promoteSkillOwnerScope`
* (#577 P1/P3) sign a skill's tamper-evident manifest with.
*
* Mirrors `auth/sessionSigningKey.ts` exactly: generate on first call,
* persist in the vault so every subsequent boot (and any replacement process
* reading the same vault) re-signs with the SAME key — a rotation here
* invalidates every previously-issued manifest signature, the same
* "log everyone out" trade-off `resolveSessionSigningKey` documents for
* cookies.
*
* A dedicated vault scope (`core:skills`), not `core:auth` — this key signs
* data-integrity artefacts, not authentication tokens. Different trust
* domain, same reasoning `credentials/crypto.ts` gives for keeping the
* credential-keychain master key separate from the provider-secret vault's:
* a single compromised key should not unlock both.
*/
export const CORE_SKILLS_AGENT_ID = 'core:skills';

const SIGNING_KEY_VAULT_KEY = 'skill_manifest_signing_key';
const KEY_BYTES = 32;

export async function resolveSkillManifestSigningKey(
vault: SecretVault,
): Promise<string> {
const existing = await vault.get(CORE_SKILLS_AGENT_ID, SIGNING_KEY_VAULT_KEY);
if (existing) return existing;
const fresh = crypto.randomBytes(KEY_BYTES).toString('hex');
await vault.set(CORE_SKILLS_AGENT_ID, SIGNING_KEY_VAULT_KEY, fresh);
return fresh;
}
101 changes: 101 additions & 0 deletions middleware/test/778RouteMounts.wiring.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
/**
* #778 W1 — composition-root wiring regression.
*
* The exact bug class this issue exists to close: `routes/credentialAsks.ts`
* (#774) and `PgSkillOwnershipLifecycleStore.promoteSkillOwnerScope` (#577
* P3) each shipped fully built and fully route/unit-tested — and stayed
* unreachable for a whole phase because nobody added the one-line
* `app.use(...)` in `src/index.ts`. A router's OWN test suite (e.g.
* `credentialAskRoutes.test.ts`, which mounts the router into its own
* throwaway `express()` app) passes identically whether or not `index.ts`
* ever mounts it — that is the "passes every route test" trap the issue
* names explicitly.
*
* `src/index.ts` runs `main().catch(...)` unconditionally at import time
* (DB pools, mDNS, plugin catalog, `app.listen`), so it cannot be imported
* or booted from a unit test without a full deployment's worth of config —
* no test in this repo does that (verified: zero references to
* `src/index.ts` from `test/**`). So this test drives the actual source
* text of the composition root instead of executing it: it is the
* deterministic half of "prove the mount," catching exactly the failure
* mode of a route file existing, fully tested standalone, but never called
* from `index.ts`. Reworded per the #470 ratchet's own guidance (never
* touch a baseline it matches) — this file matches no ratchet pattern.
*
* The second half — that the mounted router actually behaves correctly at
* runtime — is proven by `skillPromotionRoute.test.ts` (live `app.listen(0)`
* + real `fetch`, this repo's established router-test pattern; see
* `credentialAskRoutes.test.ts` and `adminProvidersRoute.test.ts`) and by
* the pre-existing `credentialAskRoutes.test.ts` for the ask surface.
*/

import { strict as assert } from 'node:assert';
import { readFileSync } from 'node:fs';
import { dirname, resolve } from 'node:path';
import { describe, it } from 'node:test';
import { fileURLToPath } from 'node:url';

const middlewareRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..');
const indexSource = readFileSync(resolve(middlewareRoot, 'src', 'index.ts'), 'utf8');

/** Strip line comments so a mount reference sitting only in a `//` doc
* comment can never satisfy this check — it must be live code. */
function withoutLineComments(src: string): string {
return src
.split('\n')
.map((line) => line.replace(/\/\/.*$/, ''))
.join('\n');
}

const liveIndexSource = withoutLineComments(indexSource);

describe('#778 W1 — index.ts actually mounts the #577/#578 routers', () => {
it('imports createSkillPromotionRouter from routes/skillPromotion.js', () => {
assert.match(
indexSource,
/import\s*\{\s*createSkillPromotionRouter\s*\}\s*from\s*'\.\/routes\/skillPromotion\.js';/,
'src/index.ts must import createSkillPromotionRouter — a route module that exists but is never imported can never be mounted',
);
});

it('mounts the skill-promotion router at /api/v1/admin/skills behind requireAuth', () => {
assert.match(
liveIndexSource,
/app\.use\(\s*'\/api\/v1\/admin\/skills',\s*requireAuth,\s*createSkillPromotionRouter\(/,
"app.use('/api/v1/admin/skills', requireAuth, createSkillPromotionRouter(...)) must appear as LIVE code in index.ts, not only in a comment",
);
});

it('imports createCredentialAskRouter from routes/credentialAsks.js', () => {
assert.match(
indexSource,
/import\s*\{\s*createCredentialAskRouter\s*\}\s*from\s*'\.\/routes\/credentialAsks\.js';/,
'src/index.ts must import createCredentialAskRouter — #774 built and route-tested this router but deliberately left it unmounted',
);
});

it('mounts the credential-asks router at /api/v1/admin/credential-asks behind requireAuth', () => {
assert.match(
liveIndexSource,
/app\.use\(\s*'\/api\/v1\/admin\/credential-asks',\s*requireAuth,\s*createCredentialAskRouter\(/,
"app.use('/api/v1/admin/credential-asks', requireAuth, createCredentialAskRouter(...)) must appear as LIVE code in index.ts, not only in a comment",
);
});

it('regression guard: fails if the skill-promotion mount line is commented out', () => {
// Proves the "strip comments" step above actually does something —
// without it, commenting out the app.use(...) line would still match
// the raw-source regex and this whole test file would be a no-op.
const withMountCommentedOut = indexSource.replace(
/app\.use\(\s*'\/api\/v1\/admin\/skills',\s*requireAuth,\s*createSkillPromotionRouter\(/,
(m) => `// ${m}`,
);
assert.notEqual(withMountCommentedOut, indexSource, 'the mount line must exist to be commented out by this check');
const strippedIfCommented = withoutLineComments(withMountCommentedOut);
assert.doesNotMatch(
strippedIfCommented,
/app\.use\(\s*'\/api\/v1\/admin\/skills',\s*requireAuth,\s*createSkillPromotionRouter\(/,
'a commented-out mount must not satisfy the live-code check',
);
});
});
Loading
Loading