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
39 changes: 39 additions & 0 deletions middleware/src/services/skillLifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,45 @@ export function isSkillOwnerScope(scope: ScopeId): scope is SkillOwnerScope {
return scope.kind === 'personal' || scope.kind === 'group' || scope.kind === 'org';
}

// ── Automation write-guard (#577 Kernkonzept #6) ────────────────────────

/**
* Thrown when a skill-mutating call is attempted by a machine actor. Carries
* the rejected `ScopeId` so a caller (route layer, log line) can report which
* automation origin was blocked without re-deriving it from the message.
*/
export class SkillAutomationWriteBlocked extends Error {
readonly actorScope: ScopeId;

constructor(actorScope: ScopeId) {
const origin = actorScope.kind === 'system' ? `:${actorScope.origin}` : '';
super(
`skill mutation blocked: actor scope is a machine origin ('${actorScope.kind}${origin}') — only a live human may modify a skill`,
);
this.name = 'SkillAutomationWriteBlocked';
this.actorScope = actorScope;
}
}

/**
* "Automations (crons) may not modify skills — as an enforced guard, not a
* convention" (#577 Kernkonzept #6). `ScopeId`'s own contract (`scopeId.ts`)
* is what makes this a one-line check rather than a new taxonomy: `kind:
* 'system'` is explicitly documented there as "no human is present in any of
* them" — routines, schedules and the Conductor's own automated runs. Every
* OTHER `ScopeId` kind — including `unscoped` — arises from an actual live
* turn, so `system` is the exact and only boundary this guard needs.
*
* Callers pass the ACTOR's scope (who/what is performing the write), never
* the skill's `ownerScope` (whose home it is) — those are unrelated axes; an
* org-owned skill can still be legitimately edited by a live human operator.
*/
export function assertHumanActor(actorScope: ScopeId): void {
if (actorScope.kind === 'system') {
throw new SkillAutomationWriteBlocked(actorScope);
}
}

// ── Lifecycle status ─────────────────────────────────────────────────────

export const SKILL_LIFECYCLE_STATUSES = ['draft', 'reviewed', 'published', 'archived'] as const;
Expand Down
86 changes: 80 additions & 6 deletions middleware/src/services/skillLifecycleStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,15 @@ import { computeSkillHash } from '@omadia/orchestrator';
import { formatSessionScope, parseSessionScope, type ScopeId } from '@omadia/channel-sdk';

import {
assertHumanActor,
canonicalSkillManifest,
isSkillOwnerScope,
requiredCapabilitiesFromFrontmatter,
signSkillManifest,
transitionSkillLifecycle,
type SkillLifecycleStatus,
type SkillLifecycleTransitionResult,
type SkillOwnerScope,
} from './skillLifecycle.js';

export interface SkillOwnershipLifecycleRow {
Expand Down Expand Up @@ -102,13 +105,22 @@ export class PgSkillOwnershipLifecycleStore {
/**
* Assign a PERSONAL owner to a still-unowned draft skill. This is the ONLY
* direct-assignment path: #577 Kernkonzept #5 forbids creating a skill
* directly in team/org scope — those homes are reached only through the
* admin-gated promotion route (P3). Refuses to reassign an already-owned
* skill (call the — not-yet-built — promotion path for that) and refuses a
* directly in team/org scope — those homes are reached only through
* {@link promoteSkillOwnerScope} (admin-gated, P3). Refuses to reassign an
* already-owned skill (call `promoteSkillOwnerScope` for that) and refuses a
* non-draft target (ownership must be settled before review begins, since
* `ownerScope` is part of what gets signed).
*
* `actorScope` is who is performing the write — checked by
* {@link assertHumanActor} (#577 Kernkonzept #6) BEFORE any query runs, so
* an automation actor never even reaches the database.
*/
async assignPersonalOwner(skillId: string, owner: Extract<ScopeId, { kind: 'personal' }>): Promise<void> {
async assignPersonalOwner(
skillId: string,
owner: Extract<ScopeId, { kind: 'personal' }>,
actorScope: ScopeId,
): Promise<void> {
assertHumanActor(actorScope);
const result = await this.pool.query(
`UPDATE skills SET owner_scope = $2, updated_at = now()
WHERE id = $1 AND owner_scope IS NULL AND lifecycle_status = 'draft'`,
Expand All @@ -127,13 +139,15 @@ export class PgSkillOwnershipLifecycleStore {
/**
* Move a skill's lifecycle status, re-signing its manifest on success.
* Throws `SkillLifecycleTransitionRejected` for every rejected move — never
* returns a "false-ish" result a caller could accidentally ignore.
* returns a "false-ish" result a caller could accidentally ignore. Checks
* {@link assertHumanActor} first, same as every other mutating method here.
*/
async transition(
skillId: string,
targetStatus: SkillLifecycleStatus,
opts: { readonly granted: ReadonlySet<string>; readonly signingKey: string },
opts: { readonly granted: ReadonlySet<string>; readonly signingKey: string; readonly actorScope: ScopeId },
): Promise<SkillOwnershipLifecycleRow> {
assertHumanActor(opts.actorScope);
const row = await this.getSkill(skillId);
if (!row) throw new Error(`skill ${skillId} not found`);
if (row.ownerScope === null) {
Expand Down Expand Up @@ -163,6 +177,66 @@ export class PgSkillOwnershipLifecycleStore {
if (!updated.rows[0]) throw new Error(`skill ${skillId} vanished during transition`);
return mapRow(updated.rows[0]);
}

/**
* Admin-gated promotion: move an ALREADY-PUBLISHED skill to a team (`group`)
* or org home. This is the only way a skill ever reaches team/org
* ownership — #577 Kernkonzept #5 forbids creating one there directly, and
* `assignPersonalOwner` above only ever assigns `personal`. Callers are
* responsible for the "admin-gated" half (an authenticated-session check at
* the route layer, P3/P4 — this method has no notion of roles); what it
* enforces itself is:
*
* - {@link assertHumanActor} — a cron may not promote a skill, same as it
* may not do anything else to one (#577 Kernkonzept #6);
* - the skill must currently be `published` — an unreviewed draft has no
* business reaching a wider audience, and archiving/promoting are not
* composable (an archived skill must be republished first, if that ever
* becomes a supported path);
* - the manifest is re-signed at the NEW `ownerScope`, same `published`
* status — `ownerScope` is a signed field (#577 P1), so a promotion IS a
* signature-changing event, not just a column update.
*/
async promoteSkillOwnerScope(
skillId: string,
targetScope: Extract<SkillOwnerScope, { kind: 'group' | 'org' }>,
opts: { readonly actorScope: ScopeId; readonly signingKey: string },
): Promise<SkillOwnershipLifecycleRow> {
assertHumanActor(opts.actorScope);
const row = await this.getSkill(skillId);
if (!row) throw new Error(`skill ${skillId} not found`);
if (row.ownerScope === null) {
throw new Error(`skill ${skillId} has no owner scope yet — assign one before promoting`);
}
if (row.lifecycleStatus !== 'published') {
throw new Error(`skill ${skillId} is not published (status: ${row.lifecycleStatus}) — only a published skill may be promoted`);
}

const newOwnerScope = formatSessionScope(targetScope);
const contentHash = computeSkillHash(row.frontmatter, row.body);
const requiredCapabilities = requiredCapabilitiesFromFrontmatter(row.frontmatter);
const signature = signSkillManifest(
{
slug: row.slug,
name: row.name,
ownerScope: newOwnerScope,
status: row.lifecycleStatus,
contentHash,
requiredCapabilities,
},
opts.signingKey,
);

const updated = await this.pool.query<SkillOwnershipLifecycleDbRow>(
`UPDATE skills SET owner_scope = $2, manifest_signature = $3, manifest_signed_at = now(), updated_at = now()
WHERE id = $1
RETURNING id, slug, name, frontmatter, body, owner_scope, lifecycle_status,
manifest_signature, manifest_signed_at`,
[skillId, newOwnerScope, signature],
);
if (!updated.rows[0]) throw new Error(`skill ${skillId} vanished during promotion`);
return mapRow(updated.rows[0]);
}
}

/** Re-exported for callers that only need to verify a row without a Pool (e.g. a webhook). */
Expand Down
95 changes: 95 additions & 0 deletions middleware/src/services/skillSharing.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* #577 P3 — sharing = a grant over `GrantStore` (#575), not a parallel ACL.
*
* Kernkonzept #5: "Sharing ist grant-basiert (`share` = ACL-Grant, `move` =
* Home-Wechsel)". This module is the one place that knows how a skill's
* "shared with me" fact is encoded as a `Capability` string, and how to turn
* a resolved capability set back into the `sharedSkillIds` P2's
* `resolveSkillByName` (`skillResolver.ts`) needs. It never touches
* `grants.ts`'s body — only its exported `GrantStore`/`resolveCapabilities`
* contract, per the #577 binding surface separation.
*
* ## Fail-closed, not "unresolved is a type" — and why THIS module differs
*
* `resolveCapabilities` (#575) treats a partial role lookup as `undefined`
* ("unresolved"), because collapsing it to an empty capability set would
* silently look like real policy on the audience-floor path where too few
* capabilities can wrongly widen who is excluded from a room. Skill sharing
* has the opposite risk shape: a grants-backend hiccup here can only ever
* HIDE a skill that would have been visible, never reveal one that
* shouldn't be — the personal/team/org buckets in `skillResolver.ts` are
* completely independent of this module and still work. So
* `resolveSharedSkillIds` still surfaces the `unresolved` fact (never lies
* about it), but `toSharedSkillIdsSet` gives callers who just want the
* resolver's `sharedSkillIds` input a one-line fail-closed default —
* "no visible shares right now" is a safe, honest degradation here in a way
* it is NOT on the audience floor.
*/

import {
resolveCapabilities,
type GrantStore,
type Principal,
type RoleSourceRegistry,
} from '@omadia/channel-sdk';

/** The `Capability` prefix that encodes "read access to this specific skill". */
const SHARED_SKILL_CAPABILITY_PREFIX = 'skill:read:';

/** The `Capability` string a grant records to share `skillId` with its holder. */
export function sharedSkillCapability(skillId: string): string {
return `${SHARED_SKILL_CAPABILITY_PREFIX}${skillId}`;
}

/** Extracts the skill id from a capability string, or `undefined` if it isn't a skill-share capability. */
export function parseSharedSkillCapability(capability: string): string | undefined {
if (!capability.startsWith(SHARED_SKILL_CAPABILITY_PREFIX)) return undefined;
const id = capability.slice(SHARED_SKILL_CAPABILITY_PREFIX.length);
return id.length > 0 ? id : undefined;
}

export type SharedSkillIdsResult =
| { readonly ok: true; readonly ids: ReadonlySet<string> }
| { readonly ok: false; readonly reason: 'unresolved' };

/**
* Resolve every skill id shared with `principal`, via `GrantStore` +
* `RoleSourceRegistry` (#575/#333) — direct grants union role grants, exactly
* as `resolveCapabilities` already defines for any other capability. Returns
* `{ ok: false, reason: 'unresolved' }` when the underlying role lookup was
* partial or the store threw — same trigger as `resolveCapabilities`
* returning `undefined` — so a caller that cares CAN distinguish "nothing is
* shared" from "we couldn't ask". A caller that doesn't care uses
* {@link toSharedSkillIdsSet}.
*/
export async function resolveSharedSkillIds(
principal: Principal,
roles: RoleSourceRegistry,
grants: GrantStore,
): Promise<SharedSkillIdsResult> {
const resolved = await resolveCapabilities(principal, roles, grants);
if (!resolved) return { ok: false, reason: 'unresolved' };

const ids = new Set<string>();
for (const capability of resolved.capabilities) {
const id = parseSharedSkillCapability(capability);
if (id !== undefined) ids.add(id);
}
// A capability granted AND denied is denied — same "denials win" rule the
// audience floor applies (`audienceFloor.ts`), applied per-id here since a
// skill share is a single-recipient grant, not a room-wide floor.
for (const capability of resolved.denials) {
const id = parseSharedSkillCapability(capability);
if (id !== undefined) ids.delete(id);
}
return { ok: true, ids };
}

/**
* Fail-closed adapter for callers (P2's `SkillResolutionContext.sharedSkillIds`)
* that want a plain set: `unresolved` collapses to empty, per this module's
* header — safe here because it can only hide a skill, never leak one.
*/
export function toSharedSkillIdsSet(result: SharedSkillIdsResult): ReadonlySet<string> {
return result.ok ? result.ids : new Set();
}
30 changes: 30 additions & 0 deletions middleware/test/skillLifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { strict as assert } from 'node:assert';

import {
SKILL_LIFECYCLE_STATUSES,
SkillAutomationWriteBlocked,
SkillManifestError,
assertHumanActor,
canPublishSkill,
canTransitionSkillLifecycle,
canonicalSkillManifest,
Expand Down Expand Up @@ -240,6 +242,34 @@ describe('signSkillManifest / verifySkillManifestSignature', () => {
});
});

// ── Automation write-guard (#577 P3, Kernkonzept #6) ────────────────────

describe('assertHumanActor', () => {
it('does not throw for any non-system ScopeId kind', () => {
assert.doesNotThrow(() => assertHumanActor({ kind: 'personal', userId: 'u1' }));
assert.doesNotThrow(() => assertHumanActor({ kind: 'group', groupRef: 'team-a' }));
assert.doesNotThrow(() => assertHumanActor({ kind: 'org', orgId: 'byte5' }));
assert.doesNotThrow(() => assertHumanActor({ kind: 'conversation', conversationId: 'c1' }));
assert.doesNotThrow(() => assertHumanActor({ kind: 'unscoped', reason: 'absent' }));
});

it('throws SkillAutomationWriteBlocked for every system origin', () => {
for (const origin of ['routine', 'schedule', 'conductor', 'conductor-builder'] as const) {
assert.throws(
() => assertHumanActor({ kind: 'system', origin, id: 'run-1' }),
(err: unknown) => {
assert.ok(err instanceof SkillAutomationWriteBlocked);
assert.match(err.message, /machine origin/);
assert.match(err.message, new RegExp(origin));
assert.deepEqual(err.actorScope, { kind: 'system', origin, id: 'run-1' });
return true;
},
origin,
);
}
});
});

// ── Combined transition decision ────────────────────────────────────────

describe('transitionSkillLifecycle', () => {
Expand Down
Loading
Loading