From 4a667a0d72519a17d1eea1c723774e6093e27d37 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:15:31 +0800 Subject: [PATCH 01/17] feat(api): add typed skill-grant model + grant algebra (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduce the canonical typed access-grant shape that will replace the legacy read-only `sharedWithUsers` / `sharedWithOrgs` allow-lists: a `SkillGrant { type, id, level }` pairing a user/org principal with a `read` or `read_write` level, plus a pure, reusable `grants.ts` helper module (effective-grant resolution, legacy derive/project, normalize). The field is added to `SkillDocument` as OPTIONAL on purpose. Skills predating #1123 carry only the legacy lists, and `effectiveGrants` derives identical READ-level grants from them when `grants` is absent — so this commit changes no behaviour and cannot disrupt existing skills. `legacyListsFromGrants` is the inverse projection used later for the transitional dual-write that keeps a rolling deploy non-disruptive. Pure machinery, no wiring yet (authz/scopeFilter/routes follow). Unit tests pin the grant algebra, especially the legacy round-trip and the read_write-wins normalization. Part of #1123 --- .../src/domains/skills/crud/grants.test.ts | 134 ++++++++++++++++++ ornn-api/src/domains/skills/crud/grants.ts | 126 ++++++++++++++++ ornn-api/src/shared/types/index.ts | 45 ++++++ 3 files changed, 305 insertions(+) create mode 100644 ornn-api/src/domains/skills/crud/grants.test.ts create mode 100644 ornn-api/src/domains/skills/crud/grants.ts diff --git a/ornn-api/src/domains/skills/crud/grants.test.ts b/ornn-api/src/domains/skills/crud/grants.test.ts new file mode 100644 index 00000000..659522fa --- /dev/null +++ b/ornn-api/src/domains/skills/crud/grants.test.ts @@ -0,0 +1,134 @@ +/** + * Unit tests for the typed access-grant helpers (#1123). + * + * Pins the grant algebra — effective-grant resolution, the legacy + * derive/project round-trip (the non-disruption invariant), and + * normalization — in isolation so the authz gates, repositories, and routes + * that build on it inherit a verified contract. + * + * @module domains/skills/crud/grants.test + */ + +import { describe, expect, it } from "bun:test"; +import type { SkillGrant } from "../../../shared/types/index"; +import { + deriveGrantsFromLegacy, + effectiveGrants, + legacyListsFromGrants, + levelAllowsWrite, + normalizeGrants, +} from "./grants"; + +describe("effectiveGrants", () => { + it("returns the explicit grants array when present", () => { + const grants: SkillGrant[] = [{ type: "user", id: "u1", level: "read_write" }]; + expect(effectiveGrants({ grants })).toBe(grants); + }); + + it("returns the explicit grants even when it is empty (an explicit no-grants state)", () => { + expect(effectiveGrants({ grants: [], sharedWithUsers: ["u1"] })).toEqual([]); + }); + + it("falls back to READ grants derived from legacy lists when grants is absent", () => { + expect( + effectiveGrants({ sharedWithUsers: ["u1"], sharedWithOrgs: ["o1"] }), + ).toEqual([ + { type: "user", id: "u1", level: "read" }, + { type: "org", id: "o1", level: "read" }, + ]); + }); + + it("treats a fully-absent source as no grants", () => { + expect(effectiveGrants({})).toEqual([]); + }); +}); + +describe("deriveGrantsFromLegacy", () => { + it("maps users + orgs to READ grants, never read_write", () => { + const out = deriveGrantsFromLegacy(["u1", "u2"], ["o1"]); + expect(out).toEqual([ + { type: "user", id: "u1", level: "read" }, + { type: "user", id: "u2", level: "read" }, + { type: "org", id: "o1", level: "read" }, + ]); + expect(out.every((g) => g.level === "read")).toBe(true); + }); + + it("dedupes and drops blank ids", () => { + expect(deriveGrantsFromLegacy(["u1", "u1", " ", ""], [])).toEqual([ + { type: "user", id: "u1", level: "read" }, + ]); + }); +}); + +describe("legacyListsFromGrants (dual-write projection)", () => { + it("places every grant id in its legacy list regardless of level (read visibility, no escalation)", () => { + const grants: SkillGrant[] = [ + { type: "user", id: "u1", level: "read" }, + { type: "user", id: "u2", level: "read_write" }, + { type: "org", id: "o1", level: "read_write" }, + ]; + expect(legacyListsFromGrants(grants)).toEqual({ + sharedWithUsers: ["u1", "u2"], + sharedWithOrgs: ["o1"], + }); + }); + + it("round-trips legacy → grants → legacy losslessly for read-only data", () => { + const lists = { sharedWithUsers: ["u1", "u2"], sharedWithOrgs: ["o1"] }; + const grants = deriveGrantsFromLegacy(lists.sharedWithUsers, lists.sharedWithOrgs); + expect(legacyListsFromGrants(grants)).toEqual(lists); + }); +}); + +describe("normalizeGrants", () => { + it("collapses duplicate (type,id) keeping the highest level (read_write wins)", () => { + expect( + normalizeGrants([ + { type: "user", id: "u1", level: "read" }, + { type: "user", id: "u1", level: "read_write" }, + ]), + ).toEqual([{ type: "user", id: "u1", level: "read_write" }]); + }); + + it("does not let a later read downgrade an earlier read_write", () => { + expect( + normalizeGrants([ + { type: "org", id: "o1", level: "read_write" }, + { type: "org", id: "o1", level: "read" }, + ]), + ).toEqual([{ type: "org", id: "o1", level: "read_write" }]); + }); + + it("trims ids and drops empties, preserving first-appearance order", () => { + expect( + normalizeGrants([ + { type: "user", id: " u2 ", level: "read" }, + { type: "user", id: "", level: "read_write" }, + { type: "user", id: "u1", level: "read" }, + ]), + ).toEqual([ + { type: "user", id: "u2", level: "read" }, + { type: "user", id: "u1", level: "read" }, + ]); + }); + + it("keeps user and org of the same id distinct", () => { + expect( + normalizeGrants([ + { type: "user", id: "x", level: "read" }, + { type: "org", id: "x", level: "read_write" }, + ]), + ).toEqual([ + { type: "user", id: "x", level: "read" }, + { type: "org", id: "x", level: "read_write" }, + ]); + }); +}); + +describe("levelAllowsWrite", () => { + it("only read_write permits writing", () => { + expect(levelAllowsWrite("read_write")).toBe(true); + expect(levelAllowsWrite("read")).toBe(false); + }); +}); diff --git a/ornn-api/src/domains/skills/crud/grants.ts b/ornn-api/src/domains/skills/crud/grants.ts new file mode 100644 index 00000000..02ceffe5 --- /dev/null +++ b/ornn-api/src/domains/skills/crud/grants.ts @@ -0,0 +1,126 @@ +/** + * Typed access-grant model helpers for skills + skillsets (#1123). + * + * Stable, reusable machinery — no I/O, no DB, no request context — so both + * the skills and skillsets domains (and their authz gates, repositories, and + * routes) converge on one definition of "what grants does this doc carry" + * and "how do typed grants map to/from the legacy read-only lists". Keeping + * it pure mirrors the `scopeFilter.ts` / `authorize.ts` split: business + * policy lives in the callers, the grant algebra lives here. + * + * Back-compat is the load-bearing invariant (#1123): a skill predating typed + * grants carries only `sharedWithUsers` / `sharedWithOrgs` (read-only). Those + * MUST resolve to identical READ access, and nobody may be silently + * escalated to write. `effectiveGrants` and `legacyListsFromGrants` enforce + * that in both directions. + * + * @module domains/skills/crud/grants + */ + +import type { + SkillGrant, + SkillPermissionLevel, +} from "../../../shared/types/index"; + +/** The legacy read-only allow-list pair carried by pre-#1123 docs. */ +export interface LegacyShareLists { + sharedWithUsers: string[]; + sharedWithOrgs: string[]; +} + +/** + * The fields needed to resolve a doc's effective grants. Both + * `SkillDocument` and `SkillsetDocument` satisfy this structurally. + */ +export interface GrantSource { + grants?: SkillGrant[] | undefined; + sharedWithUsers?: string[] | undefined; + sharedWithOrgs?: string[] | undefined; +} + +/** + * Resolve the effective typed grants for a skill / skillset. + * + * When the doc carries an explicit `grants` array that is the source of + * truth. Otherwise (legacy / un-migrated doc) derive READ-level grants from + * the legacy allow-lists so authorization is byte-for-byte identical to the + * pre-#1123 read-only behaviour. + */ +export function effectiveGrants(src: GrantSource): SkillGrant[] { + if (Array.isArray(src.grants)) return src.grants; + return deriveGrantsFromLegacy(src.sharedWithUsers ?? [], src.sharedWithOrgs ?? []); +} + +/** + * Map the legacy read-only allow-lists onto READ-level typed grants. Used by + * the boot migration and by the read-time fallback. Never produces a + * `read_write` grant — legacy sharing was read-only, so migrating it must + * not escalate anyone. + */ +export function deriveGrantsFromLegacy( + sharedWithUsers: readonly string[], + sharedWithOrgs: readonly string[], +): SkillGrant[] { + const out: SkillGrant[] = []; + for (const id of dedupe(sharedWithUsers)) out.push({ type: "user", id, level: "read" }); + for (const id of dedupe(sharedWithOrgs)) out.push({ type: "org", id, level: "read" }); + return out; +} + +/** + * Project typed grants back onto the legacy read lists for transitional + * dual-write (#1123). EVERY grant id — regardless of level — lands in the + * matching legacy list so code that only understands the read lists (an + * older pod mid-rolling-deploy) still resolves correct READ visibility for + * read_write grantees, and never escalates anyone to write (old code has no + * write concept). Dropped by the post-rollout cleanup once all pods read + * `grants`. + */ +export function legacyListsFromGrants(grants: readonly SkillGrant[]): LegacyShareLists { + const users: string[] = []; + const orgs: string[] = []; + for (const g of grants) { + if (g.type === "user") users.push(g.id); + else orgs.push(g.id); + } + return { sharedWithUsers: dedupe(users), sharedWithOrgs: dedupe(orgs) }; +} + +/** + * Normalize an incoming grants list: trim ids, drop empties, and collapse + * duplicate `(type, id)` pairs keeping the HIGHEST level (`read_write` wins + * over `read`) so a principal can never hold two conflicting grants on the + * same skill. Order is preserved by first appearance. + */ +export function normalizeGrants(grants: readonly SkillGrant[]): SkillGrant[] { + const order: string[] = []; + const byKey = new Map(); + for (const g of grants) { + const id = (g.id ?? "").trim(); + if (!id) continue; + const key = `${g.type}:${id}`; + const prev = byKey.get(key); + if (!prev) order.push(key); + const level: SkillPermissionLevel = + prev?.level === "read_write" || g.level === "read_write" ? "read_write" : "read"; + byKey.set(key, { type: g.type, id, level }); + } + return order.map((k) => byKey.get(k)!); +} + +/** True when `level` permits writing (updating content / metadata). */ +export function levelAllowsWrite(level: SkillPermissionLevel): boolean { + return level === "read_write"; +} + +function dedupe(ids: readonly string[]): string[] { + const seen = new Set(); + const out: string[] = []; + for (const raw of ids) { + const id = (raw ?? "").trim(); + if (!id || seen.has(id)) continue; + seen.add(id); + out.push(id); + } + return out; +} diff --git a/ornn-api/src/shared/types/index.ts b/ornn-api/src/shared/types/index.ts index 83e2e388..ee741241 100644 --- a/ornn-api/src/shared/types/index.ts +++ b/ornn-api/src/shared/types/index.ts @@ -71,6 +71,39 @@ export class AppError extends Error { // Skill Types // --------------------------------------------------------------------------- +/** + * Permission level a grant confers on a skill / skillset (#1123). + * - `read` → view / pull / execute / read versions. + * - `read_write` → READ plus update the skill's content & metadata. Does + * NOT include any admin / danger-zone operation (change permissions, + * transfer ownership, delete skill/version, toggle deprecation, manage + * dist-tags, bind NyxID service) — those stay with the owner (`createdBy`) + * and platform admins only. + */ +export type SkillPermissionLevel = "read" | "read_write"; + +/** The kind of principal a grant targets. */ +export type SkillGrantPrincipalType = "user" | "org"; + +/** + * One typed access grant on a skill / skillset (#1123). The canonical shape + * that replaces the legacy read-only `sharedWithUsers` / `sharedWithOrgs` + * allow-lists: every grant pairs a principal with a permission `level`. + * + * - `type` — `user` (NyxID person user_id) or `org` (NyxID org user_id). + * - `id` — the principal's NyxID id. + * - `level`— `read` or `read_write`. + * + * The author (`createdBy`) is never represented here — they hold implicit + * ADMIN. Public skills (`isPrivate === false`) are readable by everyone + * regardless of grants; `read_write` only ever applies to explicit grants. + */ +export interface SkillGrant { + type: SkillGrantPrincipalType; + id: string; + level: SkillPermissionLevel; +} + export interface SkillDocument { guid: string; name: string; @@ -113,6 +146,18 @@ export interface SkillDocument { * membership is resolved per-request via the NyxID lookup middleware. */ sharedWithOrgs: string[]; + /** + * Typed access grants (#1123) — the canonical source of truth for who can + * read vs. read-write the skill, beyond the author + platform admin. + * + * Optional for back-compat: skills created before #1123 carry only the + * legacy `sharedWithUsers` / `sharedWithOrgs` read lists. Readers MUST fall + * back to deriving read-level grants from those lists when this field is + * absent — see `effectiveGrants` in `crud/grants.ts`. The boot migration + * backfills it; the repository dual-writes the legacy lists so older code + * keeps resolving read visibility during a rolling deploy. + */ + grants?: SkillGrant[] | undefined; /** * Cached pointer to the highest version published for this skill, e.g. "1.2". * The `skill_versions` collection is the source of truth; this field exists From 1c8093c21538480a88a5efc66088c6b316f36f17 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:18:31 +0800 Subject: [PATCH 02/17] feat(api): persist + expose typed grants on skills (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the `grants` field through the skills repository and the detail response: - `create` seeds an empty `grants` array (new skills are born migrated) and keeps the legacy lists in lock-step. - `update` treats `grants` as the canonical ACL write: when provided it dual-writes the legacy `sharedWithUsers` / `sharedWithOrgs` lists via `legacyListsFromGrants` (every grant id → read visibility) so an older pod mid-rolling-deploy still resolves correct read access and nobody is escalated to write. Legacy `sharedWith*` writes only apply when `grants` is absent. - `mapDoc` surfaces stored grants with defensive coercion (drops malformed entries) and leaves the field undefined for un-migrated docs. - the skill detail response now carries `grants`, emitted via `effectiveGrants` so the shape is identical whether or not the skill has been migrated. No authorization behaviour changes yet — `grants` is only read back, the gates still use the legacy lists. Existing CRUD suite (342 tests) stays green. Part of #1123 --- .../src/domains/skills/crud/repository.ts | 74 +++++++++++++++++-- ornn-api/src/domains/skills/crud/service.ts | 5 ++ ornn-api/src/shared/types/index.ts | 6 ++ 3 files changed, 78 insertions(+), 7 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/repository.ts b/ornn-api/src/domains/skills/crud/repository.ts index 2a5f6929..c212e3c1 100644 --- a/ornn-api/src/domains/skills/crud/repository.ts +++ b/ornn-api/src/domains/skills/crud/repository.ts @@ -4,9 +4,10 @@ */ import type { Collection, Db, Document } from "mongodb"; -import type { SkillDocument, SkillMetadata } from "../../../shared/types/index"; +import type { SkillDocument, SkillGrant, SkillMetadata } from "../../../shared/types/index"; import { AppError } from "../../../shared/types/index"; import { createLogger } from "../../../shared/logger"; +import { legacyListsFromGrants } from "./grants"; // `applyScope` / `applyExtraFilters` were lifted into `scopeFilter.ts` // (#969) so the skillsets repository can reuse the exact same visibility // matrix + registry-chip filters. Re-import them here — pure move, no @@ -69,6 +70,11 @@ export interface CreateSkillData { createdByEmail?: string | undefined; createdByDisplayName?: string | undefined; isPrivate?: boolean | undefined; + /** + * Initial typed grants (#1123). Omitted for a fresh skill — new skills are + * born private with no grants; the create path seeds an empty array. + */ + grants?: SkillGrant[] | undefined; /** Initial version, e.g. "1.0". Required. */ latestVersion: string; /** Origin metadata when the skill was created via a pull from an external source. */ @@ -87,10 +93,21 @@ export interface UpdateSkillData { /** * Full replacement of the explicit per-user grant list. When undefined the * existing value on the doc is preserved. + * + * @deprecated (#1123) Prefer `grants`. Still written for the transitional + * dual-write so older pods keep resolving read visibility; the permission + * path derives these from `grants` rather than setting them directly. */ sharedWithUsers?: string[]; - /** Full replacement of the explicit per-org grant list. */ + /** Full replacement of the explicit per-org grant list. @deprecated — see `grants`. */ sharedWithOrgs?: string[]; + /** + * Full replacement of the typed grants (#1123). The canonical ACL write. + * When set, `update` also dual-writes the legacy `sharedWithUsers` / + * `sharedWithOrgs` lists (every grant id, read visibility) so a rolling + * deploy stays non-disruptive. When undefined the existing value is kept. + */ + grants?: SkillGrant[]; /** Cached latest-version pointer; update when a new package version is published. */ latestVersion?: string; /** Origin metadata; bumped by the refresh-from-source path. */ @@ -163,6 +180,8 @@ export class SkillRepository { async create(data: CreateSkillData): Promise { const now = new Date(); + const initialGrants = data.grants ?? []; + const legacy = legacyListsFromGrants(initialGrants); const doc: Record = { _id: skillId(data.guid), name: data.name, @@ -180,9 +199,12 @@ export class SkillRepository { updatedOn: now, isPrivate: data.isPrivate ?? true, // Explicit ACLs start empty — author + platform admin always have - // access; the shared-with lists are an additive allow-list. - sharedWithUsers: [], - sharedWithOrgs: [], + // access; the grants list is an additive allow-list. New skills are + // born "migrated" (#1123): an empty `grants` array plus the legacy + // lists kept in lock-step for the transitional dual-write. + grants: initialGrants, + sharedWithUsers: legacy.sharedWithUsers, + sharedWithOrgs: legacy.sharedWithOrgs, latestVersion: data.latestVersion, }; @@ -217,8 +239,20 @@ export class SkillRepository { if (data.skillHash !== undefined) setFields.skillHash = data.skillHash; if (data.storageKey !== undefined) setFields.storageKey = data.storageKey; if (data.isPrivate !== undefined) setFields.isPrivate = data.isPrivate; - if (data.sharedWithUsers !== undefined) setFields.sharedWithUsers = data.sharedWithUsers; - if (data.sharedWithOrgs !== undefined) setFields.sharedWithOrgs = data.sharedWithOrgs; + // Typed grants are the canonical write (#1123). When provided, dual-write + // the legacy lists from them (every grant id → read visibility) so an + // older pod mid-rolling-deploy still resolves correct read access. An + // explicit `sharedWith*` on the update wins only when `grants` is absent + // (legacy callers / un-migrated paths). + if (data.grants !== undefined) { + const legacy = legacyListsFromGrants(data.grants); + setFields.grants = data.grants; + setFields.sharedWithUsers = legacy.sharedWithUsers; + setFields.sharedWithOrgs = legacy.sharedWithOrgs; + } else { + if (data.sharedWithUsers !== undefined) setFields.sharedWithUsers = data.sharedWithUsers; + if (data.sharedWithOrgs !== undefined) setFields.sharedWithOrgs = data.sharedWithOrgs; + } if (data.source !== undefined) setFields.source = data.source; if (data.latestVersion !== undefined) setFields.latestVersion = data.latestVersion; @@ -842,6 +876,10 @@ function mapDoc(doc: Document | null): SkillDocument | null { // we keep it here for safety against partial migrations. sharedWithUsers: Array.isArray(doc.sharedWithUsers) ? (doc.sharedWithUsers as string[]) : [], sharedWithOrgs: Array.isArray(doc.sharedWithOrgs) ? (doc.sharedWithOrgs as string[]) : [], + // Typed grants (#1123). Absent on un-migrated docs — left undefined here + // so the doc faithfully reflects storage; callers use `effectiveGrants` + // to fall back to read-grants derived from the legacy lists. + grants: mapGrants(doc.grants), latestVersion: doc.latestVersion ?? "0.1", source: doc.source ? { @@ -886,6 +924,28 @@ function mapDoc(doc: Document | null): SkillDocument | null { }; } +/** + * Coerce the stored `grants` array defensively (#1123). Returns `undefined` + * when the field is absent (un-migrated doc) so `effectiveGrants` falls back + * to the legacy lists. Drops malformed entries (bad type/level, blank id) + * rather than trusting raw Mongo data across the trust boundary. + */ +function mapGrants(raw: unknown): SkillGrant[] | undefined { + if (!Array.isArray(raw)) return undefined; + const out: SkillGrant[] = []; + for (const entry of raw) { + if (!entry || typeof entry !== "object") continue; + const e = entry as Record; + const type = e.type; + const id = typeof e.id === "string" ? e.id.trim() : ""; + const level = e.level; + if ((type !== "user" && type !== "org") || !id) continue; + if (level !== "read" && level !== "read_write") continue; + out.push({ type, id, level }); + } + return out; +} + function mapDistTags(raw: unknown): Record | undefined { if (!raw || typeof raw !== "object") return undefined; const out: Record = {}; diff --git a/ornn-api/src/domains/skills/crud/service.ts b/ornn-api/src/domains/skills/crud/service.ts index 1f3558a3..2534bb80 100644 --- a/ornn-api/src/domains/skills/crud/service.ts +++ b/ornn-api/src/domains/skills/crud/service.ts @@ -15,6 +15,7 @@ import { fetchSkillFromGitHub, parseGithubUrl, type GitHubPullInput } from "./ut import { computeVersionDiff, type VersionDiffResult } from "./utils/versionDiff"; import { isReservedVerb } from "../../../shared/reservedVerbs"; import { canReadSkill, isMemberOfOrg, SYSTEM_ACTOR, type ActorContext } from "./authorize"; +import { effectiveGrants } from "./grants"; import { resolveClosure, type LoadVersion, @@ -1989,6 +1990,10 @@ export class SkillService { updatedOn: skill.updatedOn instanceof Date ? skill.updatedOn.toISOString() : String(skill.updatedOn), sharedWithUsers: skill.sharedWithUsers, sharedWithOrgs: skill.sharedWithOrgs, + // Canonical typed ACL (#1123). `effectiveGrants` falls back to deriving + // read grants from the legacy lists for un-migrated skills, so the + // response shape is identical regardless of migration state. + grants: effectiveGrants(skill), version, isDeprecated, deprecationNote, diff --git a/ornn-api/src/shared/types/index.ts b/ornn-api/src/shared/types/index.ts index ee741241..98f8093e 100644 --- a/ornn-api/src/shared/types/index.ts +++ b/ornn-api/src/shared/types/index.ts @@ -372,6 +372,12 @@ export interface SkillDetailResponse { sharedWithUsers: string[]; /** Org user_ids granted access — every admin/member of these orgs can read the skill. */ sharedWithOrgs: string[]; + /** + * Typed access grants (#1123) — the canonical read/read-write ACL. Always + * present in responses (the service emits `effectiveGrants(skill)` so even + * an un-migrated skill surfaces its legacy lists as read grants here). + */ + grants?: SkillGrant[] | undefined; /** * Version of this skill payload: latest when no `?version=` query was * passed, otherwise the specifically requested version. From cbea81cc7dbc3a332db9523aa35cee3f9f0b5864 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:21:21 +0800 Subject: [PATCH 03/17] feat(api): persist + expose typed grants on skillsets (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the skills grant-persistence onto the skillsets domain, which shares the ownership model verbatim: - `SkillsetDocument` + `SkillsetDetailResponse` gain the optional `grants` field; the detail builder emits `effectiveGrants(skillset)`. - the skillset repository create/update/mapDoc handle `grants` with the same canonical-write + transitional dual-write of the legacy lists. Promote the defensive stored-grant coercion into the shared `grants.ts` as `coerceStoredGrants` and use it from both repositories, removing the duplicate that briefly lived in the skills repo. Unit tests cover the malformed-entry drops and the un-migrated/empty cases. Still no authz behaviour change — grants are only read back. Part of #1123 --- .../src/domains/skills/crud/grants.test.ts | 38 +++++++++++++++++++ ornn-api/src/domains/skills/crud/grants.ts | 23 +++++++++++ .../src/domains/skills/crud/repository.ts | 26 +------------ ornn-api/src/domains/skillsets/repository.ts | 35 +++++++++++++++-- ornn-api/src/domains/skillsets/service.ts | 3 ++ ornn-api/src/domains/skillsets/types.ts | 9 +++++ 6 files changed, 106 insertions(+), 28 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/grants.test.ts b/ornn-api/src/domains/skills/crud/grants.test.ts index 659522fa..4f6ec581 100644 --- a/ornn-api/src/domains/skills/crud/grants.test.ts +++ b/ornn-api/src/domains/skills/crud/grants.test.ts @@ -12,6 +12,7 @@ import { describe, expect, it } from "bun:test"; import type { SkillGrant } from "../../../shared/types/index"; import { + coerceStoredGrants, deriveGrantsFromLegacy, effectiveGrants, legacyListsFromGrants, @@ -132,3 +133,40 @@ describe("levelAllowsWrite", () => { expect(levelAllowsWrite("read")).toBe(false); }); }); + +describe("coerceStoredGrants", () => { + it("returns undefined for a non-array (un-migrated doc)", () => { + expect(coerceStoredGrants(undefined)).toBeUndefined(); + expect(coerceStoredGrants(null)).toBeUndefined(); + expect(coerceStoredGrants("nope")).toBeUndefined(); + }); + + it("keeps well-formed entries and trims ids", () => { + expect( + coerceStoredGrants([ + { type: "user", id: " u1 ", level: "read" }, + { type: "org", id: "o1", level: "read_write" }, + ]), + ).toEqual([ + { type: "user", id: "u1", level: "read" }, + { type: "org", id: "o1", level: "read_write" }, + ]); + }); + + it("drops malformed entries (bad type, bad level, blank id, non-object)", () => { + expect( + coerceStoredGrants([ + { type: "team", id: "x", level: "read" }, + { type: "user", id: "", level: "read" }, + { type: "user", id: "u1", level: "admin" }, + 42, + null, + { type: "user", id: "u2", level: "read_write" }, + ]), + ).toEqual([{ type: "user", id: "u2", level: "read_write" }]); + }); + + it("returns an empty array for an empty stored array (explicit no-grants)", () => { + expect(coerceStoredGrants([])).toEqual([]); + }); +}); diff --git a/ornn-api/src/domains/skills/crud/grants.ts b/ornn-api/src/domains/skills/crud/grants.ts index 02ceffe5..1267edb7 100644 --- a/ornn-api/src/domains/skills/crud/grants.ts +++ b/ornn-api/src/domains/skills/crud/grants.ts @@ -113,6 +113,29 @@ export function levelAllowsWrite(level: SkillPermissionLevel): boolean { return level === "read_write"; } +/** + * Coerce a raw stored `grants` value (straight off Mongo) into a typed array, + * dropping malformed entries across the trust boundary. Returns `undefined` + * when the field is absent (un-migrated doc) so `effectiveGrants` falls back + * to the legacy lists. Shared by the skills + skillsets repositories so both + * collections coerce identically. + */ +export function coerceStoredGrants(raw: unknown): SkillGrant[] | undefined { + if (!Array.isArray(raw)) return undefined; + const out: SkillGrant[] = []; + for (const entry of raw) { + if (!entry || typeof entry !== "object") continue; + const e = entry as Record; + const type = e.type; + const id = typeof e.id === "string" ? e.id.trim() : ""; + const level = e.level; + if ((type !== "user" && type !== "org") || !id) continue; + if (level !== "read" && level !== "read_write") continue; + out.push({ type, id, level }); + } + return out; +} + function dedupe(ids: readonly string[]): string[] { const seen = new Set(); const out: string[] = []; diff --git a/ornn-api/src/domains/skills/crud/repository.ts b/ornn-api/src/domains/skills/crud/repository.ts index c212e3c1..4999f246 100644 --- a/ornn-api/src/domains/skills/crud/repository.ts +++ b/ornn-api/src/domains/skills/crud/repository.ts @@ -7,7 +7,7 @@ import type { Collection, Db, Document } from "mongodb"; import type { SkillDocument, SkillGrant, SkillMetadata } from "../../../shared/types/index"; import { AppError } from "../../../shared/types/index"; import { createLogger } from "../../../shared/logger"; -import { legacyListsFromGrants } from "./grants"; +import { coerceStoredGrants, legacyListsFromGrants } from "./grants"; // `applyScope` / `applyExtraFilters` were lifted into `scopeFilter.ts` // (#969) so the skillsets repository can reuse the exact same visibility // matrix + registry-chip filters. Re-import them here — pure move, no @@ -879,7 +879,7 @@ function mapDoc(doc: Document | null): SkillDocument | null { // Typed grants (#1123). Absent on un-migrated docs — left undefined here // so the doc faithfully reflects storage; callers use `effectiveGrants` // to fall back to read-grants derived from the legacy lists. - grants: mapGrants(doc.grants), + grants: coerceStoredGrants(doc.grants), latestVersion: doc.latestVersion ?? "0.1", source: doc.source ? { @@ -924,28 +924,6 @@ function mapDoc(doc: Document | null): SkillDocument | null { }; } -/** - * Coerce the stored `grants` array defensively (#1123). Returns `undefined` - * when the field is absent (un-migrated doc) so `effectiveGrants` falls back - * to the legacy lists. Drops malformed entries (bad type/level, blank id) - * rather than trusting raw Mongo data across the trust boundary. - */ -function mapGrants(raw: unknown): SkillGrant[] | undefined { - if (!Array.isArray(raw)) return undefined; - const out: SkillGrant[] = []; - for (const entry of raw) { - if (!entry || typeof entry !== "object") continue; - const e = entry as Record; - const type = e.type; - const id = typeof e.id === "string" ? e.id.trim() : ""; - const level = e.level; - if ((type !== "user" && type !== "org") || !id) continue; - if (level !== "read" && level !== "read_write") continue; - out.push({ type, id, level }); - } - return out; -} - function mapDistTags(raw: unknown): Record | undefined { if (!raw || typeof raw !== "object") return undefined; const out: Record = {}; diff --git a/ornn-api/src/domains/skillsets/repository.ts b/ornn-api/src/domains/skillsets/repository.ts index 36d1b527..b9b50ee6 100644 --- a/ornn-api/src/domains/skillsets/repository.ts +++ b/ornn-api/src/domains/skillsets/repository.ts @@ -15,6 +15,8 @@ import type { Collection, Db, Document } from "mongodb"; import { AppError } from "../../shared/types/index"; import { createLogger } from "../../shared/logger"; import { applyScope, applyExtraFilters, type SkillScope } from "../skills/crud/scopeFilter"; +import { coerceStoredGrants, legacyListsFromGrants } from "../skills/crud/grants"; +import type { SkillGrant } from "../../shared/types/index"; import type { SkillsetDocument, SkillsetKind } from "./types"; const logger = createLogger("skillsetRepository"); @@ -41,6 +43,8 @@ export interface CreateSkillsetData { createdByEmail?: string | undefined; createdByDisplayName?: string | undefined; isPrivate?: boolean | undefined; + /** Initial typed grants (#1123). Omitted for a fresh skillset (born empty). */ + grants?: SkillGrant[] | undefined; /** Initial version, e.g. "1.0". Required. */ latestVersion: string; } @@ -50,8 +54,15 @@ export interface UpdateSkillsetData { kind?: SkillsetKind; tags?: string[]; isPrivate?: boolean; + /** @deprecated (#1123) Prefer `grants`; still dual-written for rolling-deploy read compat. */ sharedWithUsers?: string[]; + /** @deprecated (#1123) Prefer `grants`. */ sharedWithOrgs?: string[]; + /** + * Full replacement of the typed grants (#1123). Canonical ACL write — when + * set, `update` dual-writes the legacy lists from it. Mirrors skills. + */ + grants?: SkillGrant[]; latestVersion?: string; updatedBy: string; } @@ -104,6 +115,8 @@ export class SkillsetRepository { async create(data: CreateSkillsetData): Promise { const now = new Date(); + const initialGrants = data.grants ?? []; + const legacy = legacyListsFromGrants(initialGrants); const doc: Record = { _id: skillsetId(data.guid), name: data.name, @@ -117,8 +130,10 @@ export class SkillsetRepository { updatedBy: data.createdBy, updatedOn: now, isPrivate: data.isPrivate ?? true, - sharedWithUsers: [], - sharedWithOrgs: [], + // Born "migrated" (#1123): empty typed grants + legacy lists in lock-step. + grants: initialGrants, + sharedWithUsers: legacy.sharedWithUsers, + sharedWithOrgs: legacy.sharedWithOrgs, latestVersion: data.latestVersion, }; @@ -143,8 +158,17 @@ export class SkillsetRepository { if (data.kind !== undefined) setFields.kind = data.kind; if (data.tags !== undefined) setFields.tags = data.tags; if (data.isPrivate !== undefined) setFields.isPrivate = data.isPrivate; - if (data.sharedWithUsers !== undefined) setFields.sharedWithUsers = data.sharedWithUsers; - if (data.sharedWithOrgs !== undefined) setFields.sharedWithOrgs = data.sharedWithOrgs; + // Typed grants are the canonical ACL write (#1123); dual-write legacy + // lists from them. Legacy `sharedWith*` apply only when `grants` is absent. + if (data.grants !== undefined) { + const legacy = legacyListsFromGrants(data.grants); + setFields.grants = data.grants; + setFields.sharedWithUsers = legacy.sharedWithUsers; + setFields.sharedWithOrgs = legacy.sharedWithOrgs; + } else { + if (data.sharedWithUsers !== undefined) setFields.sharedWithUsers = data.sharedWithUsers; + if (data.sharedWithOrgs !== undefined) setFields.sharedWithOrgs = data.sharedWithOrgs; + } if (data.latestVersion !== undefined) setFields.latestVersion = data.latestVersion; await this.collection.updateOne({ _id: skillsetId(guid) }, { $set: setFields }); @@ -242,6 +266,9 @@ function mapDoc(doc: Document | null): SkillsetDocument | null { isPrivate: doc.isPrivate ?? true, sharedWithUsers: Array.isArray(doc.sharedWithUsers) ? (doc.sharedWithUsers as string[]) : [], sharedWithOrgs: Array.isArray(doc.sharedWithOrgs) ? (doc.sharedWithOrgs as string[]) : [], + // Typed grants (#1123) — undefined on un-migrated docs (callers use + // `effectiveGrants` to fall back to the legacy lists). + grants: coerceStoredGrants(doc.grants), latestVersion: doc.latestVersion ?? "1.0", }; } diff --git a/ornn-api/src/domains/skillsets/service.ts b/ornn-api/src/domains/skillsets/service.ts index eab7dfdf..de9aaa75 100644 --- a/ornn-api/src/domains/skillsets/service.ts +++ b/ornn-api/src/domains/skillsets/service.ts @@ -34,6 +34,7 @@ import { } from "../skills/crud/authorize"; import type { SkillService } from "../skills/crud/service"; import { isGreater, parseVersion } from "../skills/crud/version"; +import { effectiveGrants } from "../skills/crud/grants"; import type { SkillsetRepository } from "./repository"; import type { SkillsetVersionRepository } from "./skillsetVersionRepository"; import type { @@ -453,6 +454,8 @@ function toDetail( createdByDisplayName: skillset.createdByDisplayName, sharedWithUsers: skillset.sharedWithUsers, sharedWithOrgs: skillset.sharedWithOrgs, + // Canonical typed ACL (#1123) — identical shape regardless of migration. + grants: effectiveGrants(skillset), createdOn: skillset.createdOn instanceof Date ? skillset.createdOn.toISOString() : String(skillset.createdOn), updatedOn: diff --git a/ornn-api/src/domains/skillsets/types.ts b/ornn-api/src/domains/skillsets/types.ts index fe883533..5db24ebf 100644 --- a/ornn-api/src/domains/skillsets/types.ts +++ b/ornn-api/src/domains/skillsets/types.ts @@ -22,6 +22,7 @@ */ import { z } from "zod"; +import type { SkillGrant } from "../../shared/types/index"; import { DEPENDS_ON_REF_REGEX, SKILL_NAME_REGEX, @@ -202,6 +203,12 @@ export interface SkillsetDocument { sharedWithUsers: string[]; /** Explicit per-org grants (NyxID org user_ids). */ sharedWithOrgs: string[]; + /** + * Typed access grants (#1123) — canonical read/read-write ACL, mirroring + * `SkillDocument.grants`. Optional for back-compat; readers fall back to + * deriving read grants from the legacy lists via `effectiveGrants`. + */ + grants?: SkillGrant[] | undefined; /** Cached pointer to the highest published version, e.g. "1.2". */ latestVersion: string; } @@ -255,6 +262,8 @@ export interface SkillsetDetailResponse { createdByDisplayName?: string | undefined; sharedWithUsers: string[]; sharedWithOrgs: string[]; + /** Typed access grants (#1123). Always present in responses via `effectiveGrants`. */ + grants?: SkillGrant[] | undefined; createdOn: string; updatedOn: string; } From 462b8b5ddc3c74e871163a7943a686fb1c5adf87 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:23:21 +0800 Subject: [PATCH 04/17] feat(api): boot migration backfilling typed grants (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Backfill the typed `grants` array on every skill / skillset that predates it, deriving one `read`-level grant per legacy `sharedWithUsers` / `sharedWithOrgs` entry. Runs at boot right after the model-catalog migration, before any skill read is served. Non-disruptive by construction — the hard requirement for this feature: the legacy lists are left untouched, public/private flags are untouched, and nobody is escalated to write (every backfilled grant is `read`). Idempotent: only docs missing `grants` are matched, so reruns are no-ops; it runs server-side as one updateMany + aggregation pipeline per collection. Failure is logged non-fatally because `effectiveGrants` still falls back to the legacy lists for any un-migrated doc. Integration tests run the migration against a real Mongo and pin the non-disruption invariant, the empty/ancient-doc cases, idempotency, and that an existing read_write grant is never clobbered. Part of #1123 --- ornn-api/src/bootstrap.ts | 16 +++ .../skills/crud/grants.migration.test.ts | 133 ++++++++++++++++++ .../domains/skills/crud/grants.migration.ts | 83 +++++++++++ 3 files changed, 232 insertions(+) create mode 100644 ornn-api/src/domains/skills/crud/grants.migration.test.ts create mode 100644 ornn-api/src/domains/skills/crud/grants.migration.ts diff --git a/ornn-api/src/bootstrap.ts b/ornn-api/src/bootstrap.ts index 5845e103..fee1a260 100644 --- a/ornn-api/src/bootstrap.ts +++ b/ornn-api/src/bootstrap.ts @@ -146,6 +146,7 @@ import { wireAdmin } from "./domains/admin/bootstrap"; // per-provider arrays). One-time, idempotent, runs before any // LlmProvidersService consumer reads from disk. import { migrateModelCatalogIntoProviders } from "./domains/settings/llmProviders/migration"; +import { backfillTypedGrants } from "./domains/skills/crud/grants.migration"; import { createLlmPickerRoutes } from "./domains/settings/llmProviders/routes"; // OpenAPI spec @@ -674,6 +675,21 @@ export async function bootstrap( ), ); + // ---- Typed-grants backfill (#1123) ---- + // Fold the legacy read-only `sharedWithUsers` / `sharedWithOrgs` lists into + // the typed `grants` array (every legacy grant → `read` level). One-time, + // idempotent, non-disruptive (legacy lists preserved, nobody escalated to + // write). Runs before any skill/skillset read so the authz gates + scope + // filters can rely on `grants`. Failure is non-fatal: the read-time + // fallback in `effectiveGrants` keeps un-migrated docs authorizing + // correctly off the legacy lists. + await backfillTypedGrants(db).catch((err) => + logger.error( + { err: err instanceof Error ? err.message : String(err) }, + "typed-grants backfill failed — gates fall back to legacy read lists via effectiveGrants, no data loss", + ), + ); + // The picker route — `GET /me/models?surface=...` — reads from the // per-provider arrays via `LlmProvidersService` (already constructed // upstream as part of `domains/settings/...`). The section-default diff --git a/ornn-api/src/domains/skills/crud/grants.migration.test.ts b/ornn-api/src/domains/skills/crud/grants.migration.test.ts new file mode 100644 index 00000000..3657edee --- /dev/null +++ b/ornn-api/src/domains/skills/crud/grants.migration.test.ts @@ -0,0 +1,133 @@ +/** + * Integration tests for the typed-grants boot migration (#1123). + * + * Verifies the NON-DISRUPTION invariant end-to-end against a real Mongo: + * legacy read lists become read-level grants, public/private flags and the + * legacy lists are preserved, nobody is escalated to write, and reruns are + * no-ops. + * + * @module domains/skills/crud/grants.migration.test + */ + +import { afterAll, beforeAll, beforeEach, describe, expect, it } from "bun:test"; +import { MongoMemoryServer } from "mongodb-memory-server"; +import { MongoClient, type Db } from "mongodb"; +import { backfillTypedGrants } from "./grants.migration"; + +let mongo: MongoMemoryServer; +let client: MongoClient; +let db: Db; + +beforeAll(async () => { + mongo = await MongoMemoryServer.create(); + client = new MongoClient(mongo.getUri()); + await client.connect(); + db = client.db("grants_migration_test"); +}); + +afterAll(async () => { + await client.close(); + await mongo.stop(); +}); + +beforeEach(async () => { + await db.collection("skills").deleteMany({}); + await db.collection("skillsets").deleteMany({}); +}); + +describe("backfillTypedGrants", () => { + it("derives read-level grants from the legacy lists without touching anything else", async () => { + await db.collection("skills").insertOne({ + _id: "s1" as never, + name: "legacy-private", + isPrivate: true, + sharedWithUsers: ["u1", "u2"], + sharedWithOrgs: ["o1"], + }); + + const res = await backfillTypedGrants(db); + expect(res.skillsBackfilled).toBe(1); + + const doc = await db.collection("skills").findOne({ _id: "s1" as never }); + expect(doc?.grants).toEqual([ + { type: "user", id: "u1", level: "read" }, + { type: "user", id: "u2", level: "read" }, + { type: "org", id: "o1", level: "read" }, + ]); + // Legacy lists + privacy flag preserved (non-disruptive). + expect(doc?.sharedWithUsers).toEqual(["u1", "u2"]); + expect(doc?.sharedWithOrgs).toEqual(["o1"]); + expect(doc?.isPrivate).toBe(true); + // Nobody escalated to write. + expect((doc?.grants as Array<{ level: string }>).every((g) => g.level === "read")).toBe(true); + }); + + it("backfills an empty grants array for a public skill with no shares", async () => { + await db.collection("skills").insertOne({ + _id: "s2" as never, + name: "public-no-shares", + isPrivate: false, + sharedWithUsers: [], + sharedWithOrgs: [], + }); + + await backfillTypedGrants(db); + + const doc = await db.collection("skills").findOne({ _id: "s2" as never }); + expect(doc?.grants).toEqual([]); + expect(doc?.isPrivate).toBe(false); + }); + + it("tolerates docs predating even the legacy lists", async () => { + await db.collection("skills").insertOne({ _id: "s3" as never, name: "ancient" }); + await backfillTypedGrants(db); + const doc = await db.collection("skills").findOne({ _id: "s3" as never }); + expect(doc?.grants).toEqual([]); + }); + + it("does not touch docs that already carry grants (idempotent)", async () => { + await db.collection("skills").insertOne({ + _id: "s4" as never, + name: "already-migrated", + isPrivate: true, + sharedWithUsers: ["u1"], + sharedWithOrgs: [], + grants: [{ type: "user", id: "u1", level: "read_write" }], + }); + + const res = await backfillTypedGrants(db); + expect(res.skillsBackfilled).toBe(0); + + const doc = await db.collection("skills").findOne({ _id: "s4" as never }); + // read_write grant preserved — migration must not clobber a richer ACL. + expect(doc?.grants).toEqual([{ type: "user", id: "u1", level: "read_write" }]); + }); + + it("is a no-op on a second run", async () => { + await db.collection("skills").insertOne({ + _id: "s5" as never, + name: "x", + isPrivate: true, + sharedWithUsers: ["u1"], + sharedWithOrgs: [], + }); + const first = await backfillTypedGrants(db); + expect(first.skillsBackfilled).toBe(1); + const second = await backfillTypedGrants(db); + expect(second.skillsBackfilled).toBe(0); + }); + + it("migrates skillsets the same way", async () => { + await db.collection("skillsets").insertOne({ + _id: "ss1" as never, + name: "set", + isPrivate: true, + sharedWithUsers: [], + sharedWithOrgs: ["o9"], + }); + const res = await backfillTypedGrants(db); + expect(res.skillsetsBackfilled).toBe(1); + const doc = await db.collection("skillsets").findOne({ _id: "ss1" as never }); + expect(doc?.grants).toEqual([{ type: "org", id: "o9", level: "read" }]); + }); +}); diff --git a/ornn-api/src/domains/skills/crud/grants.migration.ts b/ornn-api/src/domains/skills/crud/grants.migration.ts new file mode 100644 index 00000000..37c4292b --- /dev/null +++ b/ornn-api/src/domains/skills/crud/grants.migration.ts @@ -0,0 +1,83 @@ +/** + * Boot migration (#1123) — backfill the typed `grants` array on skills and + * skillsets created before the typed-grant model existed. + * + * Non-disruptive by construction, which is the hard requirement for this + * feature: every legacy read-only grant (`sharedWithUsers` / + * `sharedWithOrgs`) becomes a `read`-level typed grant, and the legacy lists + * are LEFT IN PLACE (the repository dual-writes them during the rolling + * deploy). Nobody is escalated to write; public skills are untouched (public + * was, and stays, read-only — visibility is governed by `isPrivate`, not by + * grants). After this runs, every doc carries `grants`, so the read/write + * gates and scope filters can rely on it. + * + * Idempotent: only docs MISSING `grants` are matched, so reruns are no-ops. + * Runs entirely server-side as one `updateMany` + aggregation pipeline per + * collection — `$map` over each doc's own legacy lists, so it scales to the + * whole collection in a single round-trip without pulling docs into the app. + * + * @module domains/skills/crud/grants.migration + */ + +import type { Db } from "mongodb"; +import { createLogger } from "../../../shared/logger"; + +const logger = createLogger("skillGrantsMigration"); + +/** The collections that carry the shared ownership/grant shape. */ +const GRANTED_COLLECTIONS = ["skills", "skillsets"] as const; + +/** + * Aggregation `$set` stage that derives `grants` from the doc's own legacy + * read lists — every entry at `read` level. `$ifNull` tolerates docs that + * predate even the legacy lists. + */ +const BUILD_GRANTS_STAGE = { + $set: { + grants: { + $concatArrays: [ + { + $map: { + input: { $ifNull: ["$sharedWithUsers", []] }, + as: "u", + in: { type: "user", id: "$$u", level: "read" }, + }, + }, + { + $map: { + input: { $ifNull: ["$sharedWithOrgs", []] }, + as: "o", + in: { type: "org", id: "$$o", level: "read" }, + }, + }, + ], + }, + }, +} as const; + +export interface GrantsMigrationResult { + skillsBackfilled: number; + skillsetsBackfilled: number; +} + +/** + * Backfill typed `grants` on every skill / skillset missing the field. + * Safe to call on every boot — see module doc. + */ +export async function backfillTypedGrants(db: Db): Promise { + const counts: Record = {}; + for (const coll of GRANTED_COLLECTIONS) { + const res = await db + .collection(coll) + .updateMany({ grants: { $exists: false } }, [BUILD_GRANTS_STAGE]); + counts[coll] = res.modifiedCount; + } + const result: GrantsMigrationResult = { + skillsBackfilled: counts.skills ?? 0, + skillsetsBackfilled: counts.skillsets ?? 0, + }; + if (result.skillsBackfilled > 0 || result.skillsetsBackfilled > 0) { + logger.info({ ...result }, "Typed-grants backfill complete (#1123)"); + } + return result; +} From bffebe2bd06e8baa57f2b32818d4bdc94e8ba92c Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:25:28 +0800 Subject: [PATCH 05/17] feat(api): split authz into read / read_write / admin tiers (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the single source-of-truth `authorize.ts` (shared by skills + skillsets) grant-aware: - `canReadSkill` now resolves access from the typed `grants` ACL — any level confers read, directly or via membership of a granted org. It falls back to `effectiveGrants` so an un-migrated doc reads exactly as before off its legacy lists. - new `canWriteSkill` = author OR platform admin OR a `read_write` grant (direct or via a granted org) — the READ_WRITE tier (content + metadata edits only). - `canManageSkill` is unchanged (author OR platform admin) = the ADMIN tier; a read_write grantee is deliberately never an admin. A shared `actorMatchesGrant` core walks the effective grants once so the read and write gates can never diverge on the matching rules; both fail soft on an unresolved org-membership lookup, matching the existing read behaviour. `SkillOwnership` gains the optional `grants` field. No route yet calls `canWriteSkill` — that re-pointing lands next. Full skills + skillsets suites (758 tests) stay green. Part of #1123 --- .../src/domains/skills/crud/authorize.test.ts | 108 +++++++++++++++++- ornn-api/src/domains/skills/crud/authorize.ts | 98 ++++++++++++---- 2 files changed, 184 insertions(+), 22 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/authorize.test.ts b/ornn-api/src/domains/skills/crud/authorize.test.ts index 4eb9ae8a..4ad9c5b1 100644 --- a/ornn-api/src/domains/skills/crud/authorize.test.ts +++ b/ornn-api/src/domains/skills/crud/authorize.test.ts @@ -14,7 +14,14 @@ import { describe, expect, it } from "bun:test"; import type { Context } from "hono"; -import { buildActorContext } from "./authorize"; +import { + buildActorContext, + canManageSkill, + canReadSkill, + canWriteSkill, + type ActorContext, + type SkillOwnership, +} from "./authorize"; import { type AuthContext, type AuthVariables, @@ -126,3 +133,102 @@ describe("buildActorContext", () => { expect(actor.memberships).toEqual([]); }); }); + +/** Build an actor; `orgs` become member-role memberships. */ +function actor(userId: string, opts: { admin?: boolean; orgs?: string[] } = {}): ActorContext { + return { + userId, + isPlatformAdmin: opts.admin === true, + memberships: (opts.orgs ?? []).map((id) => ({ userId: id, role: "member", displayName: id })), + membershipsResolved: true, + }; +} + +/** A private skill owned by `owner` carrying the given typed grants. */ +function skill(owner: string, grants: SkillOwnership["grants"], isPrivate = true): SkillOwnership { + return { createdBy: owner, isPrivate, grants, sharedWithUsers: [], sharedWithOrgs: [] }; +} + +describe("canReadSkill (#1123 tiers)", () => { + it("public skill is readable by anyone, including anonymous", () => { + expect(canReadSkill(skill("owner", [], false), actor(""))).toBe(true); + }); + + it("private skill: author, platform admin read; a stranger does not", () => { + const s = skill("owner", []); + expect(canReadSkill(s, actor("owner"))).toBe(true); + expect(canReadSkill(s, actor("x", { admin: true }))).toBe(true); + expect(canReadSkill(s, actor("stranger"))).toBe(false); + }); + + it("any grant level confers read — read AND read_write grantees can read", () => { + const s = skill("owner", [ + { type: "user", id: "reader", level: "read" }, + { type: "user", id: "editor", level: "read_write" }, + ]); + expect(canReadSkill(s, actor("reader"))).toBe(true); + expect(canReadSkill(s, actor("editor"))).toBe(true); + }); + + it("org grant confers read to a member", () => { + const s = skill("owner", [{ type: "org", id: "org-a", level: "read" }]); + expect(canReadSkill(s, actor("u", { orgs: ["org-a"] }))).toBe(true); + expect(canReadSkill(s, actor("u", { orgs: ["org-b"] }))).toBe(false); + }); + + it("falls back to legacy read lists when grants is absent (un-migrated doc)", () => { + const legacy: SkillOwnership = { + createdBy: "owner", + isPrivate: true, + sharedWithUsers: ["reader"], + sharedWithOrgs: ["org-a"], + }; + expect(canReadSkill(legacy, actor("reader"))).toBe(true); + expect(canReadSkill(legacy, actor("u", { orgs: ["org-a"] }))).toBe(true); + expect(canReadSkill(legacy, actor("stranger"))).toBe(false); + }); +}); + +describe("canWriteSkill (#1123 READ_WRITE tier)", () => { + it("author + platform admin may write", () => { + const s = skill("owner", []); + expect(canWriteSkill(s, actor("owner"))).toBe(true); + expect(canWriteSkill(s, actor("x", { admin: true }))).toBe(true); + }); + + it("a read grantee may NOT write; a read_write grantee may", () => { + const s = skill("owner", [ + { type: "user", id: "reader", level: "read" }, + { type: "user", id: "editor", level: "read_write" }, + ]); + expect(canWriteSkill(s, actor("reader"))).toBe(false); + expect(canWriteSkill(s, actor("editor"))).toBe(true); + }); + + it("a read_write org grant lets every member write", () => { + const s = skill("owner", [{ type: "org", id: "org-a", level: "read_write" }]); + expect(canWriteSkill(s, actor("u", { orgs: ["org-a"] }))).toBe(true); + expect(canWriteSkill(s, actor("u", { orgs: ["org-b"] }))).toBe(false); + }); + + it("an un-migrated (legacy-only) doc grants nobody but author/admin write", () => { + const legacy: SkillOwnership = { + createdBy: "owner", + isPrivate: true, + sharedWithUsers: ["reader"], + sharedWithOrgs: [], + }; + // Legacy lists derive to READ — so the shared reader cannot write. + expect(canWriteSkill(legacy, actor("reader"))).toBe(false); + expect(canWriteSkill(legacy, actor("owner"))).toBe(true); + }); +}); + +describe("canManageSkill (#1123 ADMIN tier)", () => { + it("only author + platform admin administer — a read_write grantee never does", () => { + const s = skill("owner", [{ type: "user", id: "editor", level: "read_write" }]); + expect(canManageSkill(s, actor("owner"))).toBe(true); + expect(canManageSkill(s, actor("x", { admin: true }))).toBe(true); + expect(canManageSkill(s, actor("editor"))).toBe(false); + }); +}); diff --git a/ornn-api/src/domains/skills/crud/authorize.ts b/ornn-api/src/domains/skills/crud/authorize.ts index 3093ae50..56cb9488 100644 --- a/ornn-api/src/domains/skills/crud/authorize.ts +++ b/ornn-api/src/domains/skills/crud/authorize.ts @@ -5,23 +5,35 @@ * topic domain all converge on these so tests only need to exercise the * policy once. * - * Read (visibility): + * Three permission tiers (#1123), all derived from one typed `grants` ACL + * (with a fallback to the legacy read-only lists for un-migrated docs — see + * `effectiveGrants` in `grants.ts`): + * + * READ — `canReadSkill`: * - PUBLIC skill → anyone. * - PRIVATE skill: * - author (`createdBy === actor.userId`) → yes - * - actor's user_id is in `sharedWithUsers` → yes - * - actor is admin/member of any org in `sharedWithOrgs` → yes * - platform admin (`ornn:admin:skill`) → yes + * - actor holds ANY grant (read or read_write), directly or via + * membership of a granted org → yes * - else → no * - * Write (update / delete / change-permissions / deprecation-toggle): + * READ_WRITE — `canWriteSkill` (update content + metadata only): + * - author → yes + * - platform admin → yes + * - actor holds a `read_write` grant, directly or via a granted org → yes + * - else → no + * + * ADMIN — `canManageSkill` (change permissions, transfer ownership, delete + * skill/version, toggle deprecation, manage dist-tags, bind NyxID service): * - author → yes * - platform admin → yes - * - else → 403 + * - else → 403. A `read_write` grantee is deliberately NOT an admin. * - * Note: org-admins no longer automatically inherit write access. If an - * author wants collaborators to edit, they can grant the user directly - * (future work — the current ACL is read-only). + * Org grants resolve uniformly: every admin/member of a granted org inherits + * the grant's level. The org-membership-based gates fail soft on an + * unresolved NyxID lookup (deny), matching the read path — they never grant + * on a "couldn't ask" result. * * @module domains/skills/crud/authorize */ @@ -33,14 +45,22 @@ import { type AuthVariables, type OrgMembershipFact, } from "../../../middleware/nyxidAuth"; +import type { SkillGrant } from "../../../shared/types/index"; +import { effectiveGrants } from "./grants"; export interface SkillOwnership { /** Author (person user_id). Always present. */ createdBy: string; isPrivate: boolean; - /** Explicit per-user allow-list. Empty = nobody extra beyond author. */ + /** + * Typed access grants (#1123) — the canonical ACL. Optional: when absent + * the gates derive read-level grants from the legacy lists below via + * `effectiveGrants`, so a pre-#1123 doc authorizes exactly as before. + */ + grants?: SkillGrant[] | undefined; + /** Legacy per-user read allow-list. Read-fallback source when `grants` absent. */ sharedWithUsers: string[]; - /** Explicit per-org allow-list. Empty = nobody extra beyond author. */ + /** Legacy per-org read allow-list. Read-fallback source when `grants` absent. */ sharedWithOrgs: string[]; } @@ -95,30 +115,66 @@ export async function buildActorContext( }; } -/** Returns true when `actor` is allowed to read the skill. */ +/** + * Returns true when `actor` is allowed to READ the skill. Any grant level + * (read or read_write) confers read — a read_write grantee is always also a + * reader. Public skills are readable by everyone. + */ export function canReadSkill(skill: SkillOwnership, actor: ActorContext): boolean { if (!skill.isPrivate) return true; if (actor.isPlatformAdmin) return true; if (skill.createdBy === actor.userId) return true; - if (skill.sharedWithUsers.includes(actor.userId)) return true; - if (skill.sharedWithOrgs.length > 0) { - for (const m of actor.memberships) { - if (skill.sharedWithOrgs.includes(m.userId)) return true; - } - } - return false; + return actorMatchesGrant(skill, actor, () => true); +} + +/** + * Returns true when `actor` may UPDATE the skill's content + metadata — the + * READ_WRITE tier (#1123). Author + platform admin always qualify; otherwise + * a `read_write` grant (direct, or via membership of a granted org) is + * required. Deliberately NOT sufficient for admin/danger-zone ops — those + * gate on `canManageSkill`. + */ +export function canWriteSkill(skill: SkillOwnership, actor: ActorContext): boolean { + if (actor.isPlatformAdmin) return true; + if (skill.createdBy === actor.userId) return true; + return actorMatchesGrant(skill, actor, (g) => g.level === "read_write"); } /** - * Returns true when `actor` is allowed to mutate the skill — update - * package, change permissions, toggle deprecation, or delete. Author-only - * plus platform admin. + * Returns true when `actor` may ADMINISTER the skill — change permissions, + * transfer ownership, delete skill/version, toggle deprecation, manage + * dist-tags, bind a NyxID service. Author + platform admin only; a + * read_write grantee can never administer. */ export function canManageSkill(skill: SkillOwnership, actor: ActorContext): boolean { if (actor.isPlatformAdmin) return true; return skill.createdBy === actor.userId; } +/** + * Shared grant-matching core: does `actor` hold a grant (passing `accept`) + * either directly as a user or via membership of a granted org? Walks the + * effective grants once so read/write gates can never diverge on the + * matching rules. Fails soft on org grants when the membership lookup was + * unresolved (actor.memberships is `[]`), matching the read path. + */ +function actorMatchesGrant( + skill: SkillOwnership, + actor: ActorContext, + accept: (grant: SkillGrant) => boolean, +): boolean { + const grants = effectiveGrants(skill); + for (const g of grants) { + if (!accept(g)) continue; + if (g.type === "user") { + if (g.id === actor.userId) return true; + } else if (actor.memberships.some((m) => m.userId === g.id)) { + return true; + } + } + return false; +} + /** * True when `actor` is currently a member (admin or member role) of the * given org. Used by the topic create path — not by skill visibility. From cd74b8bc5c01587f871e057fbd210539e1d497ec Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:34:41 +0800 Subject: [PATCH 06/17] feat(api): content edits use the read_write tier (#1123) Re-point the content/metadata update paths at the new write gate so a read_write grantee can update a skill without being its owner: - `PUT /skills/:id`: base gate becomes `canWriteSkill`; a request that actually changes `isPrivate` additionally requires `canManageSkill`, so flipping visibility stays an ADMIN-tier action (a read_write grantee can replace content but not toggle public/private). - skillset `publishVersion` gates on `canWriteSkill` (publishing a new version is a content edit); permissions/transfer/delete stay ADMIN-only. - add `canWriteSkillset` to the skillset authorize delegation + `grants` to `SkillsetOwnership`. Route tests cover the new matrix: read_write grantee republishes content (200), read_write grantee flipping visibility (403), read-only grantee republishing content (403). Part of #1123 --- .../src/domains/skills/crud/routes.test.ts | 71 +++++++++++++++++++ ornn-api/src/domains/skills/crud/routes.ts | 22 +++++- ornn-api/src/domains/skillsets/authorize.ts | 21 +++++- ornn-api/src/domains/skillsets/service.ts | 7 +- 4 files changed, 116 insertions(+), 5 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/routes.test.ts b/ornn-api/src/domains/skills/crud/routes.test.ts index 676c66e0..95576245 100644 --- a/ornn-api/src/domains/skills/crud/routes.test.ts +++ b/ornn-api/src/domains/skills/crud/routes.test.ts @@ -910,6 +910,77 @@ describe("PUT /skills/:id", () => { expect(res.status).toBe(200); expect(calls).toEqual(["updateSkill"]); }); + + test("200 ZIP republish for a read_write grantee — content edit is the write tier (#1123)", async () => { + const calls: string[] = []; + const app = buildApp({ + userId: "editor", + permissions: [UPDATE], + repo: { + findByGuid: async () => + skillDoc({ + createdBy: OWNER, + isPrivate: true, + grants: [{ type: "user", id: "editor", level: "read_write" }], + }), + }, + service: { + updateSkill: async () => { + calls.push("updateSkill"); + return detail({ createdBy: OWNER }); + }, + }, + }); + const res = await app.request("/api/v1/skills/guid-1", { + method: "PUT", + headers: { "content-type": "application/zip" }, + body: await skillZipBytes(), + }); + expect(res.status).toBe(200); + expect(calls).toEqual(["updateSkill"]); + }); + + test("403 when a read_write grantee tries to flip visibility — that is admin-only (#1123)", async () => { + const app = buildApp({ + userId: "editor", + permissions: [UPDATE], + repo: { + findByGuid: async () => + skillDoc({ + createdBy: OWNER, + isPrivate: true, + grants: [{ type: "user", id: "editor", level: "read_write" }], + }), + }, + }); + const res = await app.request("/api/v1/skills/guid-1", { + method: "PUT", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ isPrivate: false }), + }); + expect(res.status).toBe(403); + }); + + test("403 when a read-only grantee tries to republish content (#1123)", async () => { + const app = buildApp({ + userId: "reader", + permissions: [UPDATE], + repo: { + findByGuid: async () => + skillDoc({ + createdBy: OWNER, + isPrivate: true, + grants: [{ type: "user", id: "reader", level: "read" }], + }), + }, + }); + const res = await app.request("/api/v1/skills/guid-1", { + method: "PUT", + headers: { "content-type": "application/zip" }, + body: await skillZipBytes(), + }); + expect(res.status).toBe(403); + }); }); // ====================================================================== diff --git a/ornn-api/src/domains/skills/crud/routes.ts b/ornn-api/src/domains/skills/crud/routes.ts index 8014e407..bd255e6b 100644 --- a/ornn-api/src/domains/skills/crud/routes.ts +++ b/ornn-api/src/domains/skills/crud/routes.ts @@ -24,7 +24,7 @@ import { } from "../../../middleware/nyxidAuth"; import { validateBody, getValidatedBody } from "../../../middleware/validate"; import { AppError } from "../../../shared/types/index"; -import { canReadSkill, canManageSkill, buildActorContext } from "./authorize"; +import { canReadSkill, canWriteSkill, canManageSkill, buildActorContext } from "./authorize"; import { parseGithubUrl } from "./utils/githubPull"; import { rateLimit } from "../../../middleware/rateLimit"; import { createLogger } from "../../../shared/logger"; @@ -976,7 +976,10 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: throw AppError.notFound("skill_not_found", `Skill '${guid}' not found`); } const actor = await buildActorContext(c); - if (!canManageSkill(existing, actor)) { + // Updating content + metadata is the READ_WRITE tier (#1123): author, + // platform admin, or a read_write grantee. Changing `isPrivate` is a + // permission change (ADMIN tier) — gated separately below once parsed. + if (!canWriteSkill(existing, actor)) { throw AppError.forbidden( "forbidden", "You do not have permission to update this skill", @@ -1039,6 +1042,21 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: throw AppError.badRequest("no_update", "No update data provided. Send a ZIP file and/or isPrivate field."); } + // Visibility is an ADMIN-tier change (#1123): a read_write grantee may + // replace content but must not flip public/private. Enforce only when + // `isPrivate` is actually being changed so a no-op resend by an editor + // doesn't 403. + if ( + isPrivate !== undefined && + isPrivate !== existing.isPrivate && + !canManageSkill(existing, actor) + ) { + throw AppError.forbidden( + "forbidden", + "Only the skill owner or a platform admin can change a skill's visibility", + ); + } + // Zip-bomb defense (#632/#633) now runs at the service chokepoint // (`updateSkill`, only when a ZIP is actually being replaced), so it // also covers the GitHub refresh path that bypasses this route. The diff --git a/ornn-api/src/domains/skillsets/authorize.ts b/ornn-api/src/domains/skillsets/authorize.ts index 9b26d00a..0e9d4c09 100644 --- a/ornn-api/src/domains/skillsets/authorize.ts +++ b/ornn-api/src/domains/skillsets/authorize.ts @@ -12,14 +12,18 @@ import { canReadSkill, + canWriteSkill, canManageSkill, type ActorContext, } from "../skills/crud/authorize"; +import type { SkillGrant } from "../../shared/types/index"; /** Minimal ownership shape (subset of SkillsetDocument / detail). */ export interface SkillsetOwnership { createdBy: string; isPrivate: boolean; + /** Typed grants (#1123); optional — gates fall back to the legacy lists. */ + grants?: SkillGrant[] | undefined; sharedWithUsers: string[]; sharedWithOrgs: string[]; } @@ -32,7 +36,22 @@ export function canReadSkillset( return canReadSkill(skillset, actor); } -/** True when `actor` may mutate the skillset. Delegates to the skill gate. */ +/** + * True when `actor` may UPDATE the skillset's content/metadata (publish a new + * version) — the READ_WRITE tier (#1123). Delegates to the skill gate. + */ +export function canWriteSkillset( + skillset: SkillsetOwnership, + actor: ActorContext, +): boolean { + return canWriteSkill(skillset, actor); +} + +/** + * True when `actor` may ADMINISTER the skillset (permissions, transfer, + * delete) — the ADMIN tier. Author + platform admin only. Delegates to the + * skill gate. + */ export function canManageSkillset( skillset: SkillsetOwnership, actor: ActorContext, diff --git a/ornn-api/src/domains/skillsets/service.ts b/ornn-api/src/domains/skillsets/service.ts index de9aaa75..f397e5c8 100644 --- a/ornn-api/src/domains/skillsets/service.ts +++ b/ornn-api/src/domains/skillsets/service.ts @@ -27,6 +27,7 @@ import { isReservedVerb } from "../../shared/reservedVerbs"; import { resolveClosure, type ClosureNode } from "../skills/closure/resolver"; import { canReadSkill, + canWriteSkill, canManageSkill, isMemberOfOrg, SYSTEM_ACTOR, @@ -159,8 +160,10 @@ export class SkillsetService { if (!existing) { throw AppError.notFound("skillset_not_found", `Skillset '${guid}' not found`); } - if (!canManageSkill(existing, actor)) { - throw AppError.forbidden("forbidden", "You do not have permission to manage this skillset"); + // Publishing a new version is a content/metadata edit — the READ_WRITE + // tier (#1123). Permissions/transfer/delete remain ADMIN-only below. + if (!canWriteSkill(existing, actor)) { + throw AppError.forbidden("forbidden", "You do not have permission to update this skillset"); } const parsed = parseVersion(input.version); From bd268a7593ae195c4dd6852e4ca47d98195ffea2 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:40:14 +0800 Subject: [PATCH 07/17] feat(api): accept typed grants on the permissions endpoints (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Extend `PUT /skills/:id/permissions` and `PUT /skillsets/:id/permissions` to carry the canonical typed `grants` ACL, while still accepting the legacy `sharedWithUsers` / `sharedWithOrgs` arrays for backward-compatible callers (they map to READ-level grants). - add the shared `skillGrantSchema` + a `resolvePermissionGrants` helper to `grants.ts` (prefers typed grants, falls back to the legacy lists). - both permission setters now resolve to normalized grants, drop a grant that names the author (implicit ADMIN), and reuse the existing #815/#842 org-membership gate over the resolved org grants — so sharing a read_write grant into a non-member org is rejected exactly like a read share, and an unresolved lookup still 503s. - `skill.permissions_changed` analytics gains a `readWriteGrants` count. The service methods are the back-compat boundary, so the existing org-gate tests pass unchanged; assertions that checked the dual-written legacy lists now assert on the canonical `grants`. New tests cover the typed-grants path (read_write user + org), the non-member gate over an org grant, and author-self-grant pruning. Part of #1123 --- ornn-api/src/domains/skills/crud/grants.ts | 34 ++++++++ ornn-api/src/domains/skills/crud/routes.ts | 20 ++++- .../src/domains/skills/crud/service.test.ts | 77 ++++++++++++++++++- ornn-api/src/domains/skills/crud/service.ts | 35 ++++----- ornn-api/src/domains/skillsets/routes.ts | 1 + .../src/domains/skillsets/service.test.ts | 3 +- ornn-api/src/domains/skillsets/service.ts | 19 ++--- ornn-api/src/domains/skillsets/types.ts | 9 ++- 8 files changed, 162 insertions(+), 36 deletions(-) diff --git a/ornn-api/src/domains/skills/crud/grants.ts b/ornn-api/src/domains/skills/crud/grants.ts index 1267edb7..4743853e 100644 --- a/ornn-api/src/domains/skills/crud/grants.ts +++ b/ornn-api/src/domains/skills/crud/grants.ts @@ -17,11 +17,45 @@ * @module domains/skills/crud/grants */ +import { z } from "zod"; import type { SkillGrant, SkillPermissionLevel, } from "../../../shared/types/index"; +/** + * Canonical Zod schema for one typed grant on the wire (#1123). Shared by + * the skills + skillsets permission endpoints so the request shape is + * defined exactly once. Caps mirror the legacy id-length bound (128). + */ +export const skillGrantSchema = z.object({ + type: z.enum(["user", "org"]), + id: z.string().min(1).max(128), + level: z.enum(["read", "read_write"]), +}); + +/** + * A permissions request payload: the canonical `grants` array, with the + * legacy `sharedWithUsers` / `sharedWithOrgs` lists accepted for + * backward-compatibility (older SDK / API callers). + */ +export interface PermissionsPayload { + grants?: SkillGrant[] | undefined; + sharedWithUsers?: string[] | undefined; + sharedWithOrgs?: string[] | undefined; +} + +/** + * Resolve an incoming permissions payload into the canonical, normalized + * grants list. Prefers the typed `grants` field; falls back to deriving + * READ-level grants from the legacy lists so a pre-#1123 caller that still + * sends `sharedWithUsers` / `sharedWithOrgs` behaves exactly as before. + */ +export function resolvePermissionGrants(payload: PermissionsPayload): SkillGrant[] { + if (payload.grants !== undefined) return normalizeGrants(payload.grants); + return deriveGrantsFromLegacy(payload.sharedWithUsers ?? [], payload.sharedWithOrgs ?? []); +} + /** The legacy read-only allow-list pair carried by pre-#1123 docs. */ export interface LegacyShareLists { sharedWithUsers: string[]; diff --git a/ornn-api/src/domains/skills/crud/routes.ts b/ornn-api/src/domains/skills/crud/routes.ts index bd255e6b..aa017006 100644 --- a/ornn-api/src/domains/skills/crud/routes.ts +++ b/ornn-api/src/domains/skills/crud/routes.ts @@ -25,6 +25,7 @@ import { import { validateBody, getValidatedBody } from "../../../middleware/validate"; import { AppError } from "../../../shared/types/index"; import { canReadSkill, canWriteSkill, canManageSkill, buildActorContext } from "./authorize"; +import { skillGrantSchema } from "./grants"; import { parseGithubUrl } from "./utils/githubPull"; import { rateLimit } from "../../../middleware/rateLimit"; import { createLogger } from "../../../shared/logger"; @@ -35,12 +36,18 @@ const deprecationPatchSchema = z.object({ /** * Schema for `PUT /api/skills/:id/permissions`. `isPrivate === false` - * means fully public; the shared-with lists are still persisted in that - * case (no reason to wipe them — the author can flip back to private - * without losing their collaborator list). + * means fully public; the grants are still persisted in that case (no + * reason to wipe them — the author can flip back to private without losing + * their collaborator list). + * + * `grants` (#1123) is the canonical typed ACL. The legacy `sharedWithUsers` + * / `sharedWithOrgs` arrays are still accepted for backward-compatibility + * (older SDK / API callers) and map to READ-level grants when `grants` is + * omitted — see `resolvePermissionGrants`. */ const permissionsPatchSchema = z.object({ isPrivate: z.boolean(), + grants: z.array(skillGrantSchema).max(600).optional(), sharedWithUsers: z.array(z.string().min(1).max(128)).max(500).default([]), sharedWithOrgs: z.array(z.string().min(1).max(128)).max(100).default([]), }); @@ -1125,20 +1132,27 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: const body = getValidatedBody>(c); + // Pass the raw payload through — the service resolves the canonical + // `grants` (preferring the typed field, falling back to the legacy + // lists for older callers) and validates org membership. await skillService.setSkillPermissions(guid, authCtx.userId, { isPrivate: body.isPrivate, + grants: body.grants, sharedWithUsers: body.sharedWithUsers, sharedWithOrgs: body.sharedWithOrgs, }, actor); const updated = await skillService.getSkill(guid); + const readWriteGrants = (updated.grants ?? []).filter((g) => g.level === "read_write").length; trackActivity(authCtx.userId, authCtx.email, authCtx.displayName, "skill.permissions_changed", { skillId: guid, skillName: updated.name, isPrivate: updated.isPrivate, sharedWithUsers: updated.sharedWithUsers.length, sharedWithOrgs: updated.sharedWithOrgs.length, + // #1123 — richer payload: how many grants confer write. + readWriteGrants, }); // Permissions change can flip eligibility — sync handles both diff --git a/ornn-api/src/domains/skills/crud/service.test.ts b/ornn-api/src/domains/skills/crud/service.test.ts index 55ddd7fc..4197437f 100644 --- a/ornn-api/src/domains/skills/crud/service.test.ts +++ b/ornn-api/src/domains/skills/crud/service.test.ts @@ -286,7 +286,8 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = actor, ); expect(state.updateCalled).toBe(true); - expect(result.sharedWithOrgs).toContain("org-A"); + // The canonical ACL now carries the org as a read grant (#1123). + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read" }); }); it("platform admin may share into any org", async () => { @@ -303,7 +304,7 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = actor, ); expect(state.updateCalled).toBe(true); - expect(result.sharedWithOrgs).toContain("org-A"); + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read" }); }); it("a rejected share does not leak read access (AC#2)", () => { @@ -458,6 +459,78 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = // Atomic — no partial persistence of the member org. expect(state.updateCalled).toBe(false); }); + + it("(e) accepts typed grants directly, including read_write (#1123)", async () => { + const { service, state } = makePermissionsService( + makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), + ); + const actor: ActorContext = { + userId: "owner-1", + memberships: [{ userId: "org-A", role: "member", displayName: "" }], + isPlatformAdmin: false, + membershipsResolved: true, + }; + const result = await service.setSkillPermissions( + "guid-1", + "owner-1", + { + isPrivate: true, + grants: [ + { type: "user", id: "editor", level: "read_write" }, + { type: "org", id: "org-A", level: "read_write" }, + ], + }, + actor, + ); + expect(state.updateCalled).toBe(true); + expect(result.grants).toContainEqual({ type: "user", id: "editor", level: "read_write" }); + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read_write" }); + }); + + it("(f) a read_write org grant still honours the #815 non-member gate (#1123)", async () => { + const { service, state } = makePermissionsService( + makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), + ); + const actor: ActorContext = { + userId: "owner-1", + memberships: [], + isPlatformAdmin: false, + membershipsResolved: true, + }; + let thrown: unknown; + try { + await service.setSkillPermissions( + "guid-1", + "owner-1", + { isPrivate: true, grants: [{ type: "org", id: "org-Z", level: "read_write" }] }, + actor, + ); + } catch (err) { + thrown = err; + } + expect((thrown as AppError).code).toBe("not_org_member"); + expect(state.updateCalled).toBe(false); + }); + + it("(g) drops a grant that names the author themselves (#1123)", async () => { + const { service } = makePermissionsService( + makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), + ); + const actor: ActorContext = { + userId: "owner-1", + memberships: [], + isPlatformAdmin: false, + membershipsResolved: true, + }; + const result = await service.setSkillPermissions( + "guid-1", + "owner-1", + { isPrivate: true, grants: [{ type: "user", id: "owner-1", level: "read_write" }] }, + actor, + ); + // The author holds implicit ADMIN — a self-grant is redundant and dropped. + expect(result.grants).toEqual([]); + }); }); // ====================================================================== diff --git a/ornn-api/src/domains/skills/crud/service.ts b/ornn-api/src/domains/skills/crud/service.ts index 2534bb80..f0cd8c9d 100644 --- a/ornn-api/src/domains/skills/crud/service.ts +++ b/ornn-api/src/domains/skills/crud/service.ts @@ -15,7 +15,7 @@ import { fetchSkillFromGitHub, parseGithubUrl, type GitHubPullInput } from "./ut import { computeVersionDiff, type VersionDiffResult } from "./utils/versionDiff"; import { isReservedVerb } from "../../../shared/reservedVerbs"; import { canReadSkill, isMemberOfOrg, SYSTEM_ACTOR, type ActorContext } from "./authorize"; -import { effectiveGrants } from "./grants"; +import { effectiveGrants, resolvePermissionGrants, type PermissionsPayload } from "./grants"; import { resolveClosure, type LoadVersion, @@ -561,11 +561,9 @@ export class SkillService { async setSkillPermissions( guid: string, userId: string, - permissions: { - isPrivate: boolean; - sharedWithUsers: string[]; - sharedWithOrgs: string[]; - }, + // Accept either the canonical typed `grants` or the legacy lists + // (back-compat); both resolve to the same normalized grants (#1123). + permissions: { isPrivate: boolean } & PermissionsPayload, actor: ActorContext, ): Promise { const existing = await this.skillRepo.findByGuid(guid); @@ -584,15 +582,13 @@ export class SkillService { ); } - // Dedupe the lists + drop any self-references. The author always has - // access; including their id in `sharedWithUsers` is redundant and - // noisy for downstream debugging. - const sharedWithUsers = Array.from( - new Set(permissions.sharedWithUsers.filter((id) => id && id !== existing.createdBy)), - ); - const sharedWithOrgs = Array.from( - new Set(permissions.sharedWithOrgs.filter((id) => !!id)), + // Resolve to canonical normalized grants, then drop any self-reference: + // the author always has implicit ADMIN, so a grant naming them is + // redundant and noisy for downstream debugging. + const grants = resolvePermissionGrants(permissions).filter( + (g) => !(g.type === "user" && g.id === existing.createdBy), ); + const orgGrantIds = grants.filter((g) => g.type === "org").map((g) => g.id); // CWE-862 (#815): an owner may only share into orgs they belong to. // isMemberOfOrg is membership-only (does not consider platform admin), so @@ -603,9 +599,9 @@ export class SkillService { // reason. Validating a share into orgs against that empty list would // wrongly 403 a legitimate member, so fail closed with a retryable 503 // instead — but only when the caller actually asked to share into an - // org. A public / user-only change (empty sharedWithOrgs) needs no - // membership data and proceeds even while the lookup is unresolved. - if (sharedWithOrgs.length > 0 && !actor.membershipsResolved) { + // org. A public / user-only change (no org grants) needs no membership + // data and proceeds even while the lookup is unresolved. + if (orgGrantIds.length > 0 && !actor.membershipsResolved) { logger.warn( { guid, userId }, "Org membership unresolved; cannot validate share into orgs (#842)", @@ -615,7 +611,7 @@ export class SkillService { "Could not verify your organization memberships right now. Retry shortly.", ); } - const nonMember = sharedWithOrgs.filter((orgId) => !isMemberOfOrg(actor, orgId)); + const nonMember = orgGrantIds.filter((orgId) => !isMemberOfOrg(actor, orgId)); if (nonMember.length > 0) { logger.warn({ guid, userId, nonMember }, "Rejected skill share into non-member org(s) (#815)"); throw AppError.forbidden( @@ -627,8 +623,7 @@ export class SkillService { const updated = await this.skillRepo.update(guid, { isPrivate: permissions.isPrivate, - sharedWithUsers, - sharedWithOrgs, + grants, updatedBy: userId, }); return this.buildDetailResponse(updated); diff --git a/ornn-api/src/domains/skillsets/routes.ts b/ornn-api/src/domains/skillsets/routes.ts index a2d3cd4e..5a03ba2c 100644 --- a/ornn-api/src/domains/skillsets/routes.ts +++ b/ornn-api/src/domains/skillsets/routes.ts @@ -187,6 +187,7 @@ export function createSkillsetRoutes( id, { isPrivate: body.isPrivate, + grants: body.grants, sharedWithUsers: body.sharedWithUsers, sharedWithOrgs: body.sharedWithOrgs, }, diff --git a/ornn-api/src/domains/skillsets/service.test.ts b/ornn-api/src/domains/skillsets/service.test.ts index ba70d3ee..1dd422f6 100644 --- a/ornn-api/src/domains/skillsets/service.test.ts +++ b/ornn-api/src/domains/skillsets/service.test.ts @@ -415,7 +415,8 @@ describe("SkillsetService — visibility transitions (mirror skills)", () => { OWNER, ); expect(updated.isPrivate).toBe(false); - expect(updated.sharedWithUsers).toEqual(["u2"]); + // Canonical ACL now carries the user as a read grant (#1123). + expect(updated.grants).toContainEqual({ type: "user", id: "u2", level: "read" }); }); it("setPermissions 403s a non-owner", async () => { diff --git a/ornn-api/src/domains/skillsets/service.ts b/ornn-api/src/domains/skillsets/service.ts index f397e5c8..156b7034 100644 --- a/ornn-api/src/domains/skillsets/service.ts +++ b/ornn-api/src/domains/skillsets/service.ts @@ -35,7 +35,7 @@ import { } from "../skills/crud/authorize"; import type { SkillService } from "../skills/crud/service"; import { isGreater, parseVersion } from "../skills/crud/version"; -import { effectiveGrants } from "../skills/crud/grants"; +import { effectiveGrants, resolvePermissionGrants, type PermissionsPayload } from "../skills/crud/grants"; import type { SkillsetRepository } from "./repository"; import type { SkillsetVersionRepository } from "./skillsetVersionRepository"; import type { @@ -291,7 +291,9 @@ export class SkillsetService { */ async setPermissions( guid: string, - permissions: { isPrivate: boolean; sharedWithUsers: string[]; sharedWithOrgs: string[] }, + // Accept typed `grants` or the legacy lists (back-compat); both resolve + // to the same normalized grants (#1123). Mirrors the skills service. + permissions: { isPrivate: boolean } & PermissionsPayload, actor: ActorContext, ): Promise { const existing = await this.skillsetRepo.findByGuid(guid); @@ -302,20 +304,20 @@ export class SkillsetService { throw AppError.forbidden("forbidden", "You do not have permission to manage this skillset"); } - const sharedWithUsers = Array.from( - new Set(permissions.sharedWithUsers.filter((id) => id && id !== existing.createdBy)), + const grants = resolvePermissionGrants(permissions).filter( + (g) => !(g.type === "user" && g.id === existing.createdBy), ); - const sharedWithOrgs = Array.from(new Set(permissions.sharedWithOrgs.filter((id) => !!id))); + const orgGrantIds = grants.filter((g) => g.type === "org").map((g) => g.id); if (!actor.isPlatformAdmin) { - if (sharedWithOrgs.length > 0 && !actor.membershipsResolved) { + if (orgGrantIds.length > 0 && !actor.membershipsResolved) { logger.warn({ guid }, "Org membership unresolved; cannot validate share into orgs"); throw AppError.serviceUnavailable( "org_membership_unavailable", "Could not verify your organization memberships right now. Retry shortly.", ); } - const nonMember = sharedWithOrgs.filter((orgId) => !isMemberOfOrg(actor, orgId)); + const nonMember = orgGrantIds.filter((orgId) => !isMemberOfOrg(actor, orgId)); if (nonMember.length > 0) { logger.warn({ guid, nonMember }, "Rejected skillset share into non-member org(s)"); throw AppError.forbidden( @@ -327,8 +329,7 @@ export class SkillsetService { await this.skillsetRepo.update(guid, { isPrivate: permissions.isPrivate, - sharedWithUsers, - sharedWithOrgs, + grants, updatedBy: actor.userId, }); logger.info({ guid, isPrivate: permissions.isPrivate }, "Skillset permissions changed"); diff --git a/ornn-api/src/domains/skillsets/types.ts b/ornn-api/src/domains/skillsets/types.ts index 5db24ebf..3075c11d 100644 --- a/ornn-api/src/domains/skillsets/types.ts +++ b/ornn-api/src/domains/skillsets/types.ts @@ -23,6 +23,7 @@ import { z } from "zod"; import type { SkillGrant } from "../../shared/types/index"; +import { skillGrantSchema } from "../skills/crud/grants"; import { DEPENDS_ON_REF_REGEX, SKILL_NAME_REGEX, @@ -165,9 +166,15 @@ export const publishSkillsetSchema = z.object({ version: z.string().regex(SKILL_VERSION_REGEX, "version must be `.`"), }); -/** Body schema for `PUT /skillsets/:id/permissions` — mirrors skills. */ +/** + * Body schema for `PUT /skillsets/:id/permissions` — mirrors skills. `grants` + * (#1123) is the canonical typed ACL; the legacy `sharedWith*` lists are + * accepted for back-compat and map to READ-level grants when `grants` is + * omitted. + */ export const skillsetPermissionsSchema = z.object({ isPrivate: z.boolean(), + grants: z.array(skillGrantSchema).max(600).optional(), sharedWithUsers: z.array(z.string().min(1).max(128)).max(500).default([]), sharedWithOrgs: z.array(z.string().min(1).max(128)).max(100).default([]), }); From 639080620e82befde81ed1362b5a9cd676b8b989 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:45:25 +0800 Subject: [PATCH 08/17] feat(api): POST /skills/:id/transfer-ownership (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add owner-to-owner skill transfer — the first path that mutates the otherwise-immutable `createdBy`. - repository `transferOwnership`: the single explicit `createdBy` write, refreshing the cached owner labels and replacing the ACL (dual-writing legacy lists), kept separate from `update` so the immutability invariant stays obvious. - service `transferSkillOwnership`: rejects a no-op transfer to the current owner (`ownership_conflict`, 409), and recomputes grants so the prior owner is appended as a READ grantee (keeps visibility, loses edit/admin) while the new owner is dropped from any prior grant (they hold implicit ADMIN now). - route `POST /skills/:id/transfer-ownership`: ADMIN-gated (`canManageSkill` — never a read_write grant); validates the target against the injected user directory (`invalid_transfer_target`, 400, when unknown); emits the new `skill.ownership_transferred` analytics event; resyncs the mirror to refresh stale author labels. Immediate/unilateral per the agreed design. - bootstrap wires `resolveUser` from the user directory repo. Service tests cover label refresh + prior-owner read grant + new-owner drop + ownership_conflict; route tests cover the 403/404/409/400 matrix, the owner happy path, and platform-admin force-transfer. Part of #1123 --- ornn-api/src/bootstrap.ts | 4 + .../src/domains/skills/crud/repository.ts | 38 ++++++ .../src/domains/skills/crud/routes.test.ts | 103 +++++++++++++++ ornn-api/src/domains/skills/crud/routes.ts | 83 ++++++++++++ .../src/domains/skills/crud/service.test.ts | 122 ++++++++++++++++++ ornn-api/src/domains/skills/crud/service.ts | 51 +++++++- ornn-api/src/infra/analytics/index.ts | 1 + 7 files changed, 401 insertions(+), 1 deletion(-) diff --git a/ornn-api/src/bootstrap.ts b/ornn-api/src/bootstrap.ts index fee1a260..dbdc9956 100644 --- a/ornn-api/src/bootstrap.ts +++ b/ornn-api/src/bootstrap.ts @@ -767,6 +767,10 @@ export async function bootstrap( // wrapper here. extraNyxidServicesResolver: () => resolveExtraNyxidServiceNames(), mirrorService, + // #1123 — transfer-ownership target validation + owner-label refresh, + // backed by the lazily-populated user directory. + resolveUser: async (userId) => + (await userDirectoryRepo.findByUserIds([userId]))[0] ?? null, }); // ---- Domain: Skill Search ---- diff --git a/ornn-api/src/domains/skills/crud/repository.ts b/ornn-api/src/domains/skills/crud/repository.ts index 4999f246..a5836311 100644 --- a/ornn-api/src/domains/skills/crud/repository.ts +++ b/ornn-api/src/domains/skills/crud/repository.ts @@ -289,6 +289,44 @@ export class SkillRepository { logger.info({ guid, tag }, "Dist-tag deleted"); } + /** + * Reassign a skill's owner (#1123). `createdBy` is otherwise immutable — + * this is the single explicit exception, so it lives in a dedicated method + * rather than widening `UpdateSkillData`. Refreshes the cached owner-label + * fields and replaces the ACL with the caller-computed `grants` (the + * service appends the prior owner as a read grant and drops the new owner), + * dual-writing the legacy lists for rolling-deploy read compat. + */ + async transferOwnership( + guid: string, + data: { + newOwnerId: string; + newOwnerEmail: string | null; + newOwnerDisplayName: string | null; + grants: SkillGrant[]; + updatedBy: string; + }, + ): Promise { + const legacy = legacyListsFromGrants(data.grants); + await this.collection.updateOne( + { _id: skillId(guid) }, + { + $set: { + createdBy: data.newOwnerId, + createdByEmail: data.newOwnerEmail, + createdByDisplayName: data.newOwnerDisplayName, + grants: data.grants, + sharedWithUsers: legacy.sharedWithUsers, + sharedWithOrgs: legacy.sharedWithOrgs, + updatedBy: data.updatedBy, + updatedOn: new Date(), + }, + }, + ); + logger.info({ guid, newOwnerId: data.newOwnerId }, "Skill ownership transferred"); + return (await this.findByGuid(guid))!; + } + async hardDelete(guid: string): Promise { await this.collection.deleteOne({ _id: skillId(guid) }); logger.info({ guid }, "Skill hard-deleted"); diff --git a/ornn-api/src/domains/skills/crud/routes.test.ts b/ornn-api/src/domains/skills/crud/routes.test.ts index 95576245..8774c9fa 100644 --- a/ornn-api/src/domains/skills/crud/routes.test.ts +++ b/ornn-api/src/domains/skills/crud/routes.test.ts @@ -118,6 +118,9 @@ interface BuildOpts { }>; nyxidServiceClient?: { findVisibleToCaller: (...args: unknown[]) => Promise }; extraNyxidServices?: readonly string[]; + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>; } function buildApp(opts: BuildOpts = {}) { @@ -130,6 +133,7 @@ function buildApp(opts: BuildOpts = {}) { repo = {}, nyxidServiceClient, extraNyxidServices = [], + resolveUser, } = opts; const skillRepo = { @@ -145,6 +149,7 @@ function buildApp(opts: BuildOpts = {}) { nyxidServiceClient: (nyxidServiceClient ?? { findVisibleToCaller: async () => null }) as unknown as SkillRoutesConfig["nyxidServiceClient"], extraNyxidServicesResolver: async () => extraNyxidServices, + ...(resolveUser ? { resolveUser } : {}), }; const app = new Hono(); @@ -1026,6 +1031,104 @@ describe("PUT /skills/:id/permissions", () => { }); }); +// ====================================================================== +// POST /skills/:id/transfer-ownership (#1123) +// ====================================================================== + +describe("POST /skills/:id/transfer-ownership", () => { + const ALICE = { userId: "alice", email: "alice@x.io", displayName: "Alice" }; + + function transferReq(app: Hono, newOwnerUserId: string) { + return app.request("/api/v1/skills/guid-1/transfer-ownership", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ newOwnerUserId }), + }); + } + + test("403 without ornn:skill:update", async () => { + const app = buildApp({ permissions: [READ] }); + expect((await transferReq(app, "alice")).status).toBe(403); + }); + + test("404 when the skill is unknown", async () => { + const app = buildApp({ permissions: [UPDATE], repo: { findByGuid: async () => null } }); + expect((await transferReq(app, "alice")).status).toBe(404); + }); + + test("403 when the caller is not the owner / admin", async () => { + const app = buildApp({ + userId: "stranger", + permissions: [UPDATE], + repo: { findByGuid: async () => skillDoc({ createdBy: OWNER }) }, + }); + expect((await transferReq(app, "alice")).status).toBe(403); + }); + + test("409 ownership_conflict when transferring to the current owner", async () => { + const app = buildApp({ + userId: OWNER, + permissions: [UPDATE], + repo: { findByGuid: async () => skillDoc({ createdBy: OWNER }) }, + resolveUser: async () => ALICE, + }); + const res = await transferReq(app, OWNER); + expect(res.status).toBe(409); + expect(((await res.json()) as { code: string }).code).toBe("ownership_conflict"); + }); + + test("400 invalid_transfer_target when the target is not a known Ornn user", async () => { + const app = buildApp({ + userId: OWNER, + permissions: [UPDATE], + repo: { findByGuid: async () => skillDoc({ createdBy: OWNER }) }, + resolveUser: async () => null, + }); + const res = await transferReq(app, "ghost"); + expect(res.status).toBe(400); + expect(((await res.json()) as { code: string }).code).toBe("invalid_transfer_target"); + }); + + test("200 delegating to transferSkillOwnership (owner)", async () => { + const calls: string[] = []; + const app = buildApp({ + userId: OWNER, + permissions: [UPDATE], + repo: { findByGuid: async () => skillDoc({ createdBy: OWNER }) }, + resolveUser: async () => ALICE, + service: { + transferSkillOwnership: async () => { + calls.push("transferSkillOwnership"); + return detail({ createdBy: "alice" }); + }, + }, + }); + const res = await transferReq(app, "alice"); + expect(res.status).toBe(200); + expect(calls).toEqual(["transferSkillOwnership"]); + expect(((await res.json()) as { data: { skill: { createdBy: string } } }).data.skill.createdBy).toBe("alice"); + }); + + test("platform admin may force-transfer a skill they do not own", async () => { + const calls: string[] = []; + const app = buildApp({ + userId: "admin-1", + permissions: [UPDATE, "ornn:admin:skill"], + repo: { findByGuid: async () => skillDoc({ createdBy: OWNER }) }, + resolveUser: async () => ALICE, + service: { + transferSkillOwnership: async () => { + calls.push("transferSkillOwnership"); + return detail({ createdBy: "alice" }); + }, + }, + }); + const res = await transferReq(app, "alice"); + expect(res.status).toBe(200); + expect(calls).toEqual(["transferSkillOwnership"]); + }); +}); + // ====================================================================== // PUT /skills/:id/nyxid-service // ====================================================================== diff --git a/ornn-api/src/domains/skills/crud/routes.ts b/ornn-api/src/domains/skills/crud/routes.ts index aa017006..517991d2 100644 --- a/ornn-api/src/domains/skills/crud/routes.ts +++ b/ornn-api/src/domains/skills/crud/routes.ts @@ -52,6 +52,11 @@ const permissionsPatchSchema = z.object({ sharedWithOrgs: z.array(z.string().min(1).max(128)).max(100).default([]), }); +/** Body for `POST /skills/:id/transfer-ownership` (#1123). */ +const transferOwnershipSchema = z.object({ + newOwnerUserId: z.string().min(1).max(128), +}); + /** * Body for `PUT /api/v1/skills/:id/nyxid-service`. `nyxidServiceId: null` * untie; a string ties to that catalog row. The service is validated + @@ -162,6 +167,16 @@ export interface SkillRoutesConfig { * with Ornn's public + system skill set. */ mirrorService?: import("../mirror/mirrorService").MirrorService; + /** + * Resolve a userId to its directory identity (#1123). Used by + * `POST /skills/:id/transfer-ownership` to validate that the transfer + * target is a known Ornn user (signed in at least once) and to refresh + * the cached owner labels. When unset, transfer rejects every target as + * `invalid_transfer_target`. + */ + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>; } /** Marker prefix for synthetic NyxID-service ids. See `extraNyxidServices`. */ @@ -177,6 +192,7 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: nyxidServiceClient, extraNyxidServicesResolver, mirrorService, + resolveUser, } = config; const app = new Hono<{ Variables: AuthVariables }>(); @@ -1163,6 +1179,73 @@ export function createSkillRoutes(config: SkillRoutesConfig): Hono<{ Variables: }, ); + /** + * POST /skills/:id/transfer-ownership — hand the skill to another Ornn + * user (#1123). ADMIN-tier: author or platform admin only. + * + * Body: `{ newOwnerUserId }`. The target must be a known Ornn user + * (resolvable in the directory — i.e. signed in at least once) or the + * call 400s with `invalid_transfer_target`. Transfer is immediate: the + * new owner becomes the owner synchronously and the prior owner is kept + * as a READ grantee (they retain visibility, not edit/admin rights). + */ + app.post( + "/skills/:id/transfer-ownership", + auth, + requirePermission("ornn:skill:update"), + validateBody(transferOwnershipSchema, "invalid_transfer"), + async (c) => { + const guid = c.req.param("id"); + const authCtx = getAuth(c); + const body = getValidatedBody>(c); + + const existing = await skillRepo.findByGuid(guid); + if (!existing) { + throw AppError.notFound("skill_not_found", `Skill '${guid}' not found`); + } + const actor = await buildActorContext(c); + // ADMIN tier — transfer is a danger-zone op, never a read_write grant. + if (!canManageSkill(existing, actor)) { + throw AppError.forbidden( + "forbidden", + "Only the skill owner or a platform admin can transfer ownership", + ); + } + if (body.newOwnerUserId === existing.createdBy) { + throw AppError.conflict("ownership_conflict", "This user already owns the skill"); + } + + // Resolve + validate the target against the user directory. A user who + // never signed in to Ornn isn't resolvable, so reject before mutating. + const target = resolveUser ? await resolveUser(body.newOwnerUserId) : null; + if (!target) { + throw AppError.badRequest( + "invalid_transfer_target", + "Transfer target is not a known Ornn user. They must have signed in to Ornn at least once.", + ); + } + + const updated = await skillService.transferSkillOwnership( + guid, + { userId: target.userId, email: target.email, displayName: target.displayName }, + actor, + ); + + trackActivity(authCtx.userId, authCtx.email, authCtx.displayName, "skill.ownership_transferred", { + skillId: guid, + skillName: updated.name, + priorOwnerId: existing.createdBy, + newOwnerId: target.userId, + }); + + // Owner change doesn't affect mirror eligibility, but the cached author + // labels on the mirror copy are now stale — resync to refresh them. + fireMirrorSync(guid); + + return c.json({ data: { skill: updated }, error: null }); + }, + ); + /** * PUT /skills/:id/nyxid-service — Tie or untie the skill to a NyxID * catalog service. diff --git a/ornn-api/src/domains/skills/crud/service.test.ts b/ornn-api/src/domains/skills/crud/service.test.ts index 4197437f..59de2068 100644 --- a/ornn-api/src/domains/skills/crud/service.test.ts +++ b/ornn-api/src/domains/skills/crud/service.test.ts @@ -227,6 +227,24 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = state.updateCalled = true; return { ...skill, ...patch } as SkillDocument; }, + transferOwnership: async ( + _guid: string, + data: { + newOwnerId: string; + newOwnerEmail: string | null; + newOwnerDisplayName: string | null; + grants: SkillDocument["grants"]; + }, + ) => { + state.updateCalled = true; + return { + ...skill, + createdBy: data.newOwnerId, + createdByEmail: data.newOwnerEmail ?? undefined, + createdByDisplayName: data.newOwnerDisplayName ?? undefined, + grants: data.grants, + } as SkillDocument; + }, } as unknown as SkillRepository; const skillVersionRepo = { findLatestBySkill: async () => null, @@ -533,6 +551,110 @@ describe("SkillService.setSkillPermissions — org-membership gate (#815)", () = }); }); +describe("SkillService.transferSkillOwnership (#1123)", () => { + function makeTransferService(skill: SkillDocument): { + service: SkillService; + state: { updateCalled: boolean }; + } { + const state = { updateCalled: false }; + const skillRepo = { + findByGuid: async (guid: string) => (guid === skill.guid ? skill : null), + transferOwnership: async ( + _guid: string, + data: { + newOwnerId: string; + newOwnerEmail: string | null; + newOwnerDisplayName: string | null; + grants: SkillDocument["grants"]; + }, + ) => { + state.updateCalled = true; + return { + ...skill, + createdBy: data.newOwnerId, + createdByEmail: data.newOwnerEmail ?? undefined, + createdByDisplayName: data.newOwnerDisplayName ?? undefined, + grants: data.grants, + } as SkillDocument; + }, + } as unknown as SkillRepository; + const skillVersionRepo = { findLatestBySkill: async () => null } as unknown as SkillVersionRepository; + const storageClient = { + getPresignedUrl: async () => ({ presignedUrl: "http://unused", expiresAt: "" }), + } as unknown as IStorageClient; + const service = new SkillService({ + skillRepo, + skillVersionRepo, + storageClient, + storageBucketResolver: async () => "test-bucket", + }); + return { service, state }; + } + + const actor: ActorContext = { + userId: "owner-1", + memberships: [], + isPlatformAdmin: false, + membershipsResolved: true, + }; + + it("reassigns the owner, refreshes labels, and keeps the prior owner as a read grantee", async () => { + const { service, state } = makeTransferService( + makeSkillDoc({ + guid: "guid-1", + createdBy: "owner-1", + isPrivate: true, + grants: [{ type: "org", id: "org-A", level: "read_write" }], + }), + ); + const result = await service.transferSkillOwnership( + "guid-1", + { userId: "newbie", email: "new@x.io", displayName: "New Owner" }, + actor, + ); + expect(state.updateCalled).toBe(true); + expect(result.createdBy).toBe("newbie"); + expect(result.createdByEmail).toBe("new@x.io"); + // Existing org grant preserved; prior owner added as read. + expect(result.grants).toContainEqual({ type: "org", id: "org-A", level: "read_write" }); + expect(result.grants).toContainEqual({ type: "user", id: "owner-1", level: "read" }); + }); + + it("drops the new owner from any pre-existing grant (they hold implicit admin now)", async () => { + const { service } = makeTransferService( + makeSkillDoc({ + guid: "guid-1", + createdBy: "owner-1", + isPrivate: true, + grants: [{ type: "user", id: "newbie", level: "read_write" }], + }), + ); + const result = await service.transferSkillOwnership( + "guid-1", + { userId: "newbie" }, + actor, + ); + expect(result.createdBy).toBe("newbie"); + // The new owner's old grant is gone; only the prior owner's read grant remains. + expect(result.grants).toEqual([{ type: "user", id: "owner-1", level: "read" }]); + }); + + it("rejects a no-op transfer to the current owner with ownership_conflict", async () => { + const { service, state } = makeTransferService( + makeSkillDoc({ guid: "guid-1", createdBy: "owner-1", isPrivate: true }), + ); + let thrown: unknown; + try { + await service.transferSkillOwnership("guid-1", { userId: "owner-1" }, actor); + } catch (err) { + thrown = err; + } + expect((thrown as AppError).code).toBe("ownership_conflict"); + expect((thrown as AppError).statusCode).toBe(409); + expect(state.updateCalled).toBe(false); + }); +}); + // ====================================================================== // #874 — broad SkillService coverage via DI fakes (no memory-server). // diff --git a/ornn-api/src/domains/skills/crud/service.ts b/ornn-api/src/domains/skills/crud/service.ts index f0cd8c9d..027fb5ef 100644 --- a/ornn-api/src/domains/skills/crud/service.ts +++ b/ornn-api/src/domains/skills/crud/service.ts @@ -15,7 +15,7 @@ import { fetchSkillFromGitHub, parseGithubUrl, type GitHubPullInput } from "./ut import { computeVersionDiff, type VersionDiffResult } from "./utils/versionDiff"; import { isReservedVerb } from "../../../shared/reservedVerbs"; import { canReadSkill, isMemberOfOrg, SYSTEM_ACTOR, type ActorContext } from "./authorize"; -import { effectiveGrants, resolvePermissionGrants, type PermissionsPayload } from "./grants"; +import { effectiveGrants, normalizeGrants, resolvePermissionGrants, type PermissionsPayload } from "./grants"; import { resolveClosure, type LoadVersion, @@ -629,6 +629,55 @@ export class SkillService { return this.buildDetailResponse(updated); } + /** + * Transfer skill ownership to another user (#1123). + * + * The caller (route) has already gated `canManageSkill` (owner / platform + * admin) and resolved the target's identity from the user directory. Here + * we own the data mutation: reassign `createdBy`, refresh the cached owner + * labels, and recompute the ACL so the prior owner keeps READ access while + * the new owner is dropped from any grant (they now hold implicit ADMIN). + * + * Rejects a no-op transfer (target already owns it) with `ownership_conflict`. + */ + async transferSkillOwnership( + guid: string, + newOwner: { userId: string; email?: string | undefined; displayName?: string | undefined }, + actor: ActorContext, + ): Promise { + const existing = await this.skillRepo.findByGuid(guid); + if (!existing) { + throw AppError.notFound("skill_not_found", `Skill '${guid}' not found`); + } + if (newOwner.userId === existing.createdBy) { + throw AppError.conflict("ownership_conflict", "This user already owns the skill"); + } + + // New ACL: keep every existing grant EXCEPT one naming the new owner + // (they become owner → implicit ADMIN), and append the prior owner as a + // READ grantee so they retain visibility but not edit/admin rights. + const priorOwner = existing.createdBy; + const grants = normalizeGrants([ + ...effectiveGrants(existing).filter( + (g) => !(g.type === "user" && g.id === newOwner.userId), + ), + { type: "user", id: priorOwner, level: "read" }, + ]); + + const updated = await this.skillRepo.transferOwnership(guid, { + newOwnerId: newOwner.userId, + newOwnerEmail: newOwner.email ?? null, + newOwnerDisplayName: newOwner.displayName ?? null, + grants, + updatedBy: actor.userId, + }); + logger.info( + { guid, priorOwner, newOwnerId: newOwner.userId, by: actor.userId }, + "Skill ownership transferred", + ); + return this.buildDetailResponse(updated); + } + async updateSkill( guid: string, userId: string, diff --git a/ornn-api/src/infra/analytics/index.ts b/ornn-api/src/infra/analytics/index.ts index c5cf6c2d..46e5b067 100644 --- a/ornn-api/src/infra/analytics/index.ts +++ b/ornn-api/src/infra/analytics/index.ts @@ -59,6 +59,7 @@ export type PlatformActivityAction = | "skill.version_deleted" | "skill.visibility_changed" | "skill.permissions_changed" + | "skill.ownership_transferred" | "skill.refresh" | "skill.nyxid_service_tied" | "skill.source_linked" From 763b762da1f018d7ec3e408f0b9ef32861def5d6 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:51:17 +0800 Subject: [PATCH 09/17] feat(api): POST /skillsets/:id/transfer-ownership (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror skill transfer onto skillsets. Because the skillset routes delegate authorization to the service (the route layer has no repo), the service owns the whole flow — ADMIN gate (`canManageSkill`), no-op rejection (`ownership_conflict`), AND target resolution against the injected user directory. Resolving inside the service (rather than the route) means a non-owner can't enumerate users via the 400-vs-403 difference. The repository gets the same dedicated `transferOwnership` write (the one explicit `createdBy` mutation), refreshing owner labels + replacing the ACL with the prior owner kept as a READ grantee. `resolveUser` is injected into `SkillsetService` via `wireSkillsets`, backed by the same user-directory repo as the skills path. Service tests cover owner reassignment + prior-owner read grant, the non-owner 403, the no-op 409, and the unresolvable-target 400; route tests cover the permission gate, delegation, and body validation. Part of #1123 --- ornn-api/src/bootstrap.ts | 8 +- ornn-api/src/domains/skillsets/bootstrap.ts | 5 + ornn-api/src/domains/skillsets/repository.ts | 35 ++++++ ornn-api/src/domains/skillsets/routes.test.ts | 40 +++++++ ornn-api/src/domains/skillsets/routes.ts | 27 +++++ .../src/domains/skillsets/service.test.ts | 105 +++++++++++++++++- ornn-api/src/domains/skillsets/service.ts | 78 ++++++++++++- 7 files changed, 294 insertions(+), 4 deletions(-) diff --git a/ornn-api/src/bootstrap.ts b/ornn-api/src/bootstrap.ts index dbdc9956..59f6b9eb 100644 --- a/ornn-api/src/bootstrap.ts +++ b/ornn-api/src/bootstrap.ts @@ -790,7 +790,13 @@ export async function bootstrap( // A skillset is a curated, versioned meta-package over N member skills. // The service injects `skillService` so member resolution + the #968 // closure walk stay single-sourced. - const skillsets = wireSkillsets({ db, skillService }); + const skillsets = wireSkillsets({ + db, + skillService, + // #1123 — transfer-ownership target validation, shared resolver. + resolveUser: async (userId) => + (await userDirectoryRepo.findByUserIds([userId]))[0] ?? null, + }); await skillsets.ensureIndexes(); // ---- Domain: Skill Generation ---- diff --git a/ornn-api/src/domains/skillsets/bootstrap.ts b/ornn-api/src/domains/skillsets/bootstrap.ts index 20644fe5..e476e741 100644 --- a/ornn-api/src/domains/skillsets/bootstrap.ts +++ b/ornn-api/src/domains/skillsets/bootstrap.ts @@ -31,6 +31,10 @@ export interface SkillsetWiring { export function wireSkillsets(deps: { db: Db; skillService: SkillService; + /** #1123 — directory resolver for ownership-transfer target validation. */ + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>; }): SkillsetWiring { const skillsetRepo = new SkillsetRepository(deps.db); const skillsetVersionRepo = new SkillsetVersionRepository(deps.db); @@ -39,6 +43,7 @@ export function wireSkillsets(deps: { skillsetRepo, skillsetVersionRepo, skillService: deps.skillService, + ...(deps.resolveUser ? { resolveUser: deps.resolveUser } : {}), }); const routes = createSkillsetRoutes({ skillsetService: service }); diff --git a/ornn-api/src/domains/skillsets/repository.ts b/ornn-api/src/domains/skillsets/repository.ts index b9b50ee6..b92bd32a 100644 --- a/ornn-api/src/domains/skillsets/repository.ts +++ b/ornn-api/src/domains/skillsets/repository.ts @@ -176,6 +176,41 @@ export class SkillsetRepository { return (await this.findByGuid(guid))!; } + /** + * Reassign a skillset's owner (#1123). Mirrors `SkillRepository`: the + * single explicit `createdBy` write, refreshing cached owner labels and + * replacing the ACL with the caller-computed grants (dual-writing legacy). + */ + async transferOwnership( + guid: string, + data: { + newOwnerId: string; + newOwnerEmail: string | null; + newOwnerDisplayName: string | null; + grants: SkillGrant[]; + updatedBy: string; + }, + ): Promise { + const legacy = legacyListsFromGrants(data.grants); + await this.collection.updateOne( + { _id: skillsetId(guid) }, + { + $set: { + createdBy: data.newOwnerId, + createdByEmail: data.newOwnerEmail, + createdByDisplayName: data.newOwnerDisplayName, + grants: data.grants, + sharedWithUsers: legacy.sharedWithUsers, + sharedWithOrgs: legacy.sharedWithOrgs, + updatedBy: data.updatedBy, + updatedOn: new Date(), + }, + }, + ); + logger.info({ guid, newOwnerId: data.newOwnerId }, "Skillset ownership transferred"); + return (await this.findByGuid(guid))!; + } + async hardDelete(guid: string): Promise { await this.collection.deleteOne({ _id: skillsetId(guid) }); logger.info({ guid }, "Skillset hard-deleted"); diff --git a/ornn-api/src/domains/skillsets/routes.test.ts b/ornn-api/src/domains/skillsets/routes.test.ts index 0e6bb52d..3bf6e5d5 100644 --- a/ornn-api/src/domains/skillsets/routes.test.ts +++ b/ornn-api/src/domains/skillsets/routes.test.ts @@ -276,4 +276,44 @@ describe("PUT/DELETE /skillsets/:id — scope gating", () => { const res = await app.request("/api/v1/skillsets/ss-1", { method: "DELETE" }); expect(res.status).toBe(200); }); + + test("transfer-ownership 403 without ornn:skill:update", async () => { + const app = buildApp({ permissions: [] }); + const res = await app.request("/api/v1/skillsets/ss-1/transfer-ownership", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ newOwnerUserId: "alice" }), + }); + expect(res.status).toBe(403); + }); + + test("transfer-ownership 200 delegating to the service", async () => { + const calls: string[] = []; + const app = buildApp({ + permissions: [UPDATE], + service: { + transferOwnership: async () => { + calls.push("transferOwnership"); + return { guid: "ss-1", createdBy: "alice" }; + }, + }, + }); + const res = await app.request("/api/v1/skillsets/ss-1/transfer-ownership", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ newOwnerUserId: "alice" }), + }); + expect(res.status).toBe(200); + expect(calls).toEqual(["transferOwnership"]); + }); + + test("transfer-ownership 400 on a missing newOwnerUserId", async () => { + const app = buildApp({ permissions: [UPDATE] }); + const res = await app.request("/api/v1/skillsets/ss-1/transfer-ownership", { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({}), + }); + expect(res.status).toBe(400); + }); }); diff --git a/ornn-api/src/domains/skillsets/routes.ts b/ornn-api/src/domains/skillsets/routes.ts index 5a03ba2c..7bf56dd2 100644 --- a/ornn-api/src/domains/skillsets/routes.ts +++ b/ornn-api/src/domains/skillsets/routes.ts @@ -49,6 +49,11 @@ export interface SkillsetRoutesConfig { skillsetService: SkillsetService; } +/** Body for `POST /skillsets/:id/transfer-ownership` (#1123). */ +const transferOwnershipSchema = z.object({ + newOwnerUserId: z.string().min(1).max(128), +}); + /** Anonymous read actor — sees public skillsets only. Fresh per call so * the mutable `memberships` array is never shared. */ function anonActor(): ActorContext { @@ -213,5 +218,27 @@ export function createSkillsetRoutes( }, ); + /** + * POST /skillsets/:id/transfer-ownership — hand the skillset to another + * Ornn user (#1123). ADMIN-tier; immediate; prior owner kept as READ. + * Mirrors the skills endpoint. + */ + app.post( + "/skillsets/:id/transfer-ownership", + auth, + requirePermission("ornn:skill:update"), + validateBody(transferOwnershipSchema, "invalid_transfer"), + async (c) => { + const id = c.req.param("id"); + const body = getValidatedBody>(c); + const actor = await buildActorContext(c); + // The service owns the full flow (ADMIN gate + target validation + + // mutation) so a non-owner can't enumerate users via the response. + const updated = await skillsetService.transferOwnership(id, body.newOwnerUserId, actor); + logger.info({ guid: id, newOwnerId: body.newOwnerUserId }, "Skillset ownership transferred via API"); + return c.json({ data: { skillset: updated }, error: null }); + }, + ); + return app; } diff --git a/ornn-api/src/domains/skillsets/service.test.ts b/ornn-api/src/domains/skillsets/service.test.ts index 1dd422f6..743d6df4 100644 --- a/ornn-api/src/domains/skillsets/service.test.ts +++ b/ornn-api/src/domains/skillsets/service.test.ts @@ -115,7 +115,12 @@ interface SkillsetState { versions: SkillsetVersionDocument[]; } -function makeSkillsetDeps(skillService: SkillService) { +function makeSkillsetDeps( + skillService: SkillService, + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>, +) { const state: SkillsetState = { skillsets: new Map(), byName: new Map(), @@ -161,6 +166,28 @@ function makeSkillsetDeps(skillService: SkillService) { state.byName.set(next.name, next); return next; }, + transferOwnership: async ( + g: string, + data: { + newOwnerId: string; + newOwnerEmail: string | null; + newOwnerDisplayName: string | null; + grants: SkillsetDocument["grants"]; + }, + ) => { + const cur = state.skillsets.get(g)!; + const next = { + ...cur, + createdBy: data.newOwnerId, + createdByEmail: data.newOwnerEmail ?? undefined, + createdByDisplayName: data.newOwnerDisplayName ?? undefined, + grants: data.grants, + updatedOn: new Date(), + } as SkillsetDocument; + state.skillsets.set(g, next); + state.byName.set(next.name, next); + return next; + }, hardDelete: async (g: string) => { const doc = state.skillsets.get(g); if (doc) state.byName.delete(doc.name); @@ -221,7 +248,7 @@ function makeSkillsetDeps(skillService: SkillService) { } as unknown as import("./skillsetVersionRepository").SkillsetVersionRepository; return { - deps: { skillsetRepo, skillsetVersionRepo, skillService }, + deps: { skillsetRepo, skillsetVersionRepo, skillService, ...(resolveUser ? { resolveUser } : {}) }, state, }; } @@ -456,6 +483,80 @@ describe("SkillsetService — visibility transitions (mirror skills)", () => { }); }); +describe("SkillsetService.transferOwnership (#1123)", () => { + const ALICE = { userId: "alice", email: "alice@x.io", displayName: "Alice" }; + + async function seedSkillset( + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>, + ) { + const { skills, versions } = twoMemberSkills(); + const skillService = makeSkillService(skills, versions); + const { deps, state } = makeSkillsetDeps(skillService, resolveUser); + const service = new SkillsetService(deps); + const created = await service.createSkillset( + { + name: "review-set", + description: "d", + instructions: "p", + kind: "generic", + tags: [], + members: ["pdf-tools@1.0", "csv-tools@1.0"], + version: "1.0", + }, + { userId: "owner-1" }, + ); + return { service, state, guid: created.guid }; + } + + it("reassigns the owner and keeps the prior owner as a read grantee", async () => { + const { service, guid } = await seedSkillset(async () => ALICE); + const updated = await service.transferOwnership(guid, "alice", OWNER); + expect(updated.createdBy).toBe("alice"); + expect(updated.grants).toContainEqual({ type: "user", id: "owner-1", level: "read" }); + }); + + it("403s a non-owner", async () => { + const { service, guid } = await seedSkillset(async () => ALICE); + const stranger: ActorContext = { + userId: "stranger", + memberships: [], + isPlatformAdmin: false, + membershipsResolved: true, + }; + let code = ""; + try { + await service.transferOwnership(guid, "alice", stranger); + } catch (err) { + code = (err as AppError).code; + } + expect(code).toBe("forbidden"); + }); + + it("409s a no-op transfer to the current owner", async () => { + const { service, guid } = await seedSkillset(async () => ALICE); + let code = ""; + try { + await service.transferOwnership(guid, "owner-1", OWNER); + } catch (err) { + code = (err as AppError).code; + } + expect(code).toBe("ownership_conflict"); + }); + + it("400s an unresolvable transfer target", async () => { + const { service, guid } = await seedSkillset(async () => null); + let code = ""; + try { + await service.transferOwnership(guid, "ghost", OWNER); + } catch (err) { + code = (err as AppError).code; + } + expect(code).toBe("invalid_transfer_target"); + }); +}); + describe("SkillsetService — publish member validation (#969)", () => { it("rejects a non-existent member with skill_dependency_not_found", async () => { const { skills, versions } = twoMemberSkills(); diff --git a/ornn-api/src/domains/skillsets/service.ts b/ornn-api/src/domains/skillsets/service.ts index 156b7034..23ec65f7 100644 --- a/ornn-api/src/domains/skillsets/service.ts +++ b/ornn-api/src/domains/skillsets/service.ts @@ -35,7 +35,12 @@ import { } from "../skills/crud/authorize"; import type { SkillService } from "../skills/crud/service"; import { isGreater, parseVersion } from "../skills/crud/version"; -import { effectiveGrants, resolvePermissionGrants, type PermissionsPayload } from "../skills/crud/grants"; +import { + effectiveGrants, + normalizeGrants, + resolvePermissionGrants, + type PermissionsPayload, +} from "../skills/crud/grants"; import type { SkillsetRepository } from "./repository"; import type { SkillsetVersionRepository } from "./skillsetVersionRepository"; import type { @@ -68,17 +73,29 @@ export interface SkillsetServiceDeps { skillsetVersionRepo: SkillsetVersionRepository; /** Injected to reuse the member-ref loader + closure resolution (#968). */ skillService: SkillService; + /** + * Resolve a userId to its directory identity (#1123). Used by ownership + * transfer to validate the target is a known Ornn user. When unset, + * transfer rejects every target as `invalid_transfer_target`. + */ + resolveUser?: ( + userId: string, + ) => Promise<{ userId: string; email: string; displayName: string } | null>; } export class SkillsetService { private readonly skillsetRepo: SkillsetRepository; private readonly skillsetVersionRepo: SkillsetVersionRepository; private readonly skillService: SkillService; + private readonly resolveUser?: + | ((userId: string) => Promise<{ userId: string; email: string; displayName: string } | null>) + | undefined; constructor(deps: SkillsetServiceDeps) { this.skillsetRepo = deps.skillsetRepo; this.skillsetVersionRepo = deps.skillsetVersionRepo; this.skillService = deps.skillService; + this.resolveUser = deps.resolveUser; } // ========================================================================== @@ -336,6 +353,65 @@ export class SkillsetService { return this.getSkillset(guid); } + /** + * Transfer skillset ownership to another user (#1123). Mirrors + * `SkillService.transferSkillOwnership`, but — because the skillset routes + * delegate authorization to the service — this method owns the full flow: + * ADMIN gate (`canManageSkill`), no-op rejection (`ownership_conflict`), + * target validation against the directory (`invalid_transfer_target`, + * resolved internally so a non-owner can't enumerate users), then the + * mutation (reassign `createdBy`, refresh labels, keep the prior owner as + * a READ grantee, drop the new owner from any prior grant). + */ + async transferOwnership( + guid: string, + newOwnerUserId: string, + actor: ActorContext, + ): Promise { + const existing = await this.skillsetRepo.findByGuid(guid); + if (!existing) { + throw AppError.notFound("skillset_not_found", `Skillset '${guid}' not found`); + } + if (!canManageSkill(existing, actor)) { + throw AppError.forbidden( + "forbidden", + "Only the skillset owner or a platform admin can transfer ownership", + ); + } + if (newOwnerUserId === existing.createdBy) { + throw AppError.conflict("ownership_conflict", "This user already owns the skillset"); + } + + const target = this.resolveUser ? await this.resolveUser(newOwnerUserId) : null; + if (!target) { + throw AppError.badRequest( + "invalid_transfer_target", + "Transfer target is not a known Ornn user. They must have signed in to Ornn at least once.", + ); + } + + const priorOwner = existing.createdBy; + const grants = normalizeGrants([ + ...effectiveGrants(existing).filter( + (g) => !(g.type === "user" && g.id === target.userId), + ), + { type: "user", id: priorOwner, level: "read" }, + ]); + + await this.skillsetRepo.transferOwnership(guid, { + newOwnerId: target.userId, + newOwnerEmail: target.email, + newOwnerDisplayName: target.displayName, + grants, + updatedBy: actor.userId, + }); + logger.info( + { guid, priorOwner, newOwnerId: target.userId, by: actor.userId }, + "Skillset ownership transferred", + ); + return this.getSkillset(guid); + } + // ========================================================================== // Closure (one-call resolve — roots = members) // ========================================================================== From 477c0078df5af063f7f986c142f339ef93400195 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:53:22 +0800 Subject: [PATCH 10/17] docs: changeset for skill permissions + ownership transfer (#1123) Minor bump for ornn-api + ornn-web (fixed-version pair). The SDKs release on a separate cadence and are not part of the changeset set. Part of #1123 --- .changeset/skill-permissions-ownership-transfer.md | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 .changeset/skill-permissions-ownership-transfer.md diff --git a/.changeset/skill-permissions-ownership-transfer.md b/.changeset/skill-permissions-ownership-transfer.md new file mode 100644 index 00000000..5d099abd --- /dev/null +++ b/.changeset/skill-permissions-ownership-transfer.md @@ -0,0 +1,10 @@ +--- +"ornn-api": minor +"ornn-web": minor +--- + +Skill & skillset permission levels (read / read-write) and ownership transfer. + +Skills and skillsets now carry a typed `grants` ACL where each user/org grant has a level: **read** (view / pull / execute) or **read-write** (also update the skill's content & metadata). Read-write grantees cannot manage permissions, transfer ownership, or delete — those admin/danger-zone actions stay with the owner and platform admins only. + +Owners (and platform admins) can **transfer a skill or skillset to another Ornn user** from the Danger Zone; the transfer is immediate and the prior owner keeps read access. All existing shares (public, org, and per-user) migrate to read-only with no disruption to current access. From 5416ec31ce35d3b02af13f619ebca29f258c098c Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 01:56:28 +0800 Subject: [PATCH 11/17] feat(web): data layer for typed grants + ownership transfer (#1123) Frontend plumbing for the new API surface, no UI yet: - `SkillPermissionLevel` + `SkillGrant` domain types; `grants?` added to `SkillDetail`. - `permissionsApi`: `SkillPermissionsInput.grants` (legacy `sharedWith*` arrays kept optional for back-compat) + a `transferSkillOwnership` POST. - `useTransferSkillOwnership(skillGuid, idOrName)` mutation following the established #750 two-id split + invalidation (detail, lists, my-skills, shared-with-me) so the new owner redraws and the owner-only UI drops after a transfer. Part of #1123 --- ornn-web/src/hooks/useSkills.ts | 26 ++++++++++++++++++- ornn-web/src/services/permissionsApi.ts | 34 ++++++++++++++++++++++--- ornn-web/src/types/domain.ts | 20 +++++++++++++++ 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/ornn-web/src/hooks/useSkills.ts b/ornn-web/src/hooks/useSkills.ts index a0229687..82b5f32b 100644 --- a/ornn-web/src/hooks/useSkills.ts +++ b/ornn-web/src/hooks/useSkills.ts @@ -17,7 +17,11 @@ import { tieSkillToNyxidService, type PullFromGitHubInput, } from "@/services/skillApi"; -import { updateSkillPermissions, type SkillPermissionsInput } from "@/services/permissionsApi"; +import { + updateSkillPermissions, + transferSkillOwnership, + type SkillPermissionsInput, +} from "@/services/permissionsApi"; import type { SkillSearchParams, SystemFilter } from "@/types/search"; import type { UpdateSkillMetadata } from "@/types/api"; @@ -348,6 +352,26 @@ export function useUpdateSkillPermissions(idOrName: string) { }); } +/** + * Transfer skill ownership to another Ornn user (#1123). Wired with the + * two-id split (#750): `skillGuid` for the write, `idOrName` for cache keys. + * On success the caller is no longer the owner, so the detail + list caches + * are invalidated to redraw the new owner and drop the owner-only UI; the + * Shared-with-me tab is invalidated too since the caller becomes a grantee. + */ +export function useTransferSkillOwnership(skillGuid: string, idOrName: string) { + const queryClient = useQueryClient(); + return useMutation({ + mutationFn: (newOwnerUserId: string) => transferSkillOwnership(skillGuid, newOwnerUserId), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: [SKILLS_KEY, idOrName] }); + queryClient.invalidateQueries({ queryKey: [SKILLS_KEY] }); + queryClient.invalidateQueries({ queryKey: [MY_SKILLS_KEY] }); + queryClient.invalidateQueries({ queryKey: [SHARED_WITH_ME_KEY] }); + }, + }); +} + /** * Tie or untie a skill to a NyxID catalog service. Invalidates the * registry tabs (especially System) and the skill detail cache so the diff --git a/ornn-web/src/services/permissionsApi.ts b/ornn-web/src/services/permissionsApi.ts index 25f437b3..a9cf5d77 100644 --- a/ornn-web/src/services/permissionsApi.ts +++ b/ornn-web/src/services/permissionsApi.ts @@ -6,13 +6,19 @@ * @module services/permissionsApi */ -import { apiPut } from "./apiClient"; -import type { SkillDetail } from "@/types/domain"; +import { apiPut, apiPost } from "./apiClient"; +import type { SkillDetail, SkillGrant } from "@/types/domain"; export interface SkillPermissionsInput { isPrivate: boolean; - sharedWithUsers: string[]; - sharedWithOrgs: string[]; + /** + * Canonical typed ACL (#1123). When provided the backend uses it directly. + * The legacy `sharedWith*` arrays remain accepted for callers that haven't + * migrated and map to READ-level grants. + */ + grants?: SkillGrant[]; + sharedWithUsers?: string[]; + sharedWithOrgs?: string[]; } export interface SkillPermissionsResult { @@ -29,3 +35,23 @@ export async function updateSkillPermissions( ); return res.data!; } + +export interface TransferSkillOwnershipResult { + skill: SkillDetail; +} + +/** + * Transfer a skill to another Ornn user (#1123). Owner / platform-admin only; + * the target must be a known Ornn user. Returns the refreshed detail — the + * caller is no longer the owner on success. + */ +export async function transferSkillOwnership( + skillGuid: string, + newOwnerUserId: string, +): Promise { + const res = await apiPost( + `/api/v1/skills/${encodeURIComponent(skillGuid)}/transfer-ownership`, + { newOwnerUserId }, + ); + return res.data!; +} diff --git a/ornn-web/src/types/domain.ts b/ornn-web/src/types/domain.ts index c4b085fc..4819702d 100644 --- a/ornn-web/src/types/domain.ts +++ b/ornn-web/src/types/domain.ts @@ -40,6 +40,20 @@ export type SkillSource = lastSyncedCommit?: string; }; +/** + * Permission level a grant confers (#1123). `read` = view/pull/execute; + * `read_write` = also update the skill's content & metadata (no admin — + * permission management, transfer, and delete stay with the owner + admins). + */ +export type SkillPermissionLevel = "read" | "read_write"; + +/** One typed access grant on a skill / skillset (#1123). */ +export interface SkillGrant { + type: "user" | "org"; + id: string; + level: SkillPermissionLevel; +} + export interface SkillDetail extends SkillSummary { updatedOn: string; presignedPackageUrl: string; @@ -54,6 +68,12 @@ export interface SkillDetail extends SkillSummary { sharedWithUsers: string[]; /** Org user_ids this skill has been explicitly shared with. */ sharedWithOrgs: string[]; + /** + * Typed access grants (#1123) — the canonical read/read-write ACL. Always + * present in API responses; the legacy `sharedWith*` arrays above mirror + * the read-visibility of these for back-compat. + */ + grants?: SkillGrant[]; /** Present when the skill was created (or last refreshed) by pulling from an external source. */ source?: SkillSource; /** Optional license string from `SKILL.md` frontmatter (e.g. "MIT"). */ From a7ff56f671998a93f7aecfb843acf31aa869a3fb Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 02:02:49 +0800 Subject: [PATCH 12/17] feat(web): Transfer ownership action in the skill Danger Zone (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a "Transfer ownership" action above Delete in the owner-only Danger Zone. `TransferOwnershipModal` reuses the directory email-typeahead (the same `searchUsersByEmail` source the PermissionsModal uses, NOT the admin-only picker) for a single-target select, then requires the owner to type the skill name before the danger-styled Transfer button enables. On success the caller is no longer the owner, so the existing list/detail invalidation in `useTransferSkillOwnership` refetches the detail — the new owner renders and the owner-only Danger Zone drops away. Modal state lives in `useSkillDetail` alongside the other modal flags. Tests cover the two-step gate (target + exact name) and that confirming fires the mutation with the selected user id. Part of #1123 --- .../skill/TransferOwnershipModal.test.tsx | 137 +++++++++++ .../skill/TransferOwnershipModal.tsx | 215 ++++++++++++++++++ ornn-web/src/hooks/useSkillDetail.ts | 3 + ornn-web/src/pages/skill/SkillDetailPage.tsx | 23 ++ 4 files changed, 378 insertions(+) create mode 100644 ornn-web/src/components/skill/TransferOwnershipModal.test.tsx create mode 100644 ornn-web/src/components/skill/TransferOwnershipModal.tsx diff --git a/ornn-web/src/components/skill/TransferOwnershipModal.test.tsx b/ornn-web/src/components/skill/TransferOwnershipModal.test.tsx new file mode 100644 index 00000000..23b59233 --- /dev/null +++ b/ornn-web/src/components/skill/TransferOwnershipModal.test.tsx @@ -0,0 +1,137 @@ +/** + * TransferOwnershipModal tests (#1123). + * + * Guards the two-step safety flow: a target must be picked from the + * directory AND the skill name typed exactly before the transfer can fire, + * and the mutation is called with the selected user's id. + * + * Directory search + mutation + toast are mocked; framer-motion (Modal + * AnimatePresence) is stubbed; react-i18next is global. + * + * @module components/skill/TransferOwnershipModal.test + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { SkillDetail } from "@/types/domain"; + +const mutateAsync = vi.fn(); +const addToast = vi.fn(); +const searchUsersByEmail = vi.fn(); + +vi.mock("framer-motion", () => ({ + AnimatePresence: ({ children }: { children: React.ReactNode }) => <>{children}, + motion: new Proxy( + {}, + { + get: (_t, tag: string) => { + const Comp: React.FC & { children?: React.ReactNode }> = ({ + children, + initial: _i, + animate: _a, + exit: _e, + transition: _tr, + ...rest + }) => { + void _i; void _a; void _e; void _tr; + const Tag = tag as keyof React.JSX.IntrinsicElements; + return {children}; + }; + return Comp; + }, + }, + ), +})); + +vi.mock("@/hooks/useSkills", () => ({ + useTransferSkillOwnership: () => ({ mutateAsync, isPending: false }), +})); + +vi.mock("@/stores/toastStore", () => ({ + useToastStore: (selector: (s: { addToast: typeof addToast }) => T) => selector({ addToast }), +})); + +vi.mock("@/services/usersApi", () => ({ + searchUsersByEmail: (...args: unknown[]) => searchUsersByEmail(...args), +})); + +import { TransferOwnershipModal } from "./TransferOwnershipModal"; + +function skill(overrides: Partial = {}): SkillDetail { + return { + guid: "skill-guid", + name: "demo-skill", + description: "", + createdBy: "owner-1", + createdOn: "2026-05-01T00:00:00.000Z", + isPrivate: true, + tags: [], + updatedOn: "2026-05-01T00:00:00.000Z", + presignedPackageUrl: "", + metadata: {}, + version: "1.0", + sharedWithUsers: [], + sharedWithOrgs: [], + ...overrides, + } as SkillDetail; +} + +function renderModal() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + {}} skill={skill()} /> + , + ); +} + +const transferButton = () => screen.getByRole("button", { name: /^transfer ownership$/i }); + +beforeEach(() => { + mutateAsync.mockReset().mockResolvedValue({ skill: skill({ createdBy: "alice-id" }) }); + addToast.mockReset(); + searchUsersByEmail.mockReset().mockResolvedValue([ + { userId: "alice-id", email: "alice@x.io", displayName: "Alice" }, + ]); +}); + +afterEach(() => cleanup()); + +describe("TransferOwnershipModal", () => { + it("disables transfer until a target is picked and the name is confirmed", async () => { + renderModal(); + expect(transferButton()).toBeDisabled(); + + // Pick a target from the directory typeahead. + fireEvent.focus(screen.getByPlaceholderText(/type an email/i)); + fireEvent.change(screen.getByPlaceholderText(/type an email/i), { target: { value: "al" } }); + const suggestion = await screen.findByText("Alice"); + fireEvent.mouseDown(suggestion); + + // Target chosen but name not yet typed → still disabled. + expect(transferButton()).toBeDisabled(); + + // Type the WRONG name → still disabled. + const confirmInput = screen.getByPlaceholderText("demo-skill"); + fireEvent.change(confirmInput, { target: { value: "wrong-name" } }); + expect(transferButton()).toBeDisabled(); + + // Type the exact name → enabled. + fireEvent.change(confirmInput, { target: { value: "demo-skill" } }); + expect(transferButton()).not.toBeDisabled(); + }); + + it("fires the mutation with the selected user id on confirm", async () => { + renderModal(); + fireEvent.focus(screen.getByPlaceholderText(/type an email/i)); + fireEvent.change(screen.getByPlaceholderText(/type an email/i), { target: { value: "al" } }); + fireEvent.mouseDown(await screen.findByText("Alice")); + fireEvent.change(screen.getByPlaceholderText("demo-skill"), { target: { value: "demo-skill" } }); + + fireEvent.click(transferButton()); + + await waitFor(() => expect(mutateAsync).toHaveBeenCalledWith("alice-id")); + await waitFor(() => expect(addToast).toHaveBeenCalledWith(expect.objectContaining({ type: "success" }))); + }); +}); diff --git a/ornn-web/src/components/skill/TransferOwnershipModal.tsx b/ornn-web/src/components/skill/TransferOwnershipModal.tsx new file mode 100644 index 00000000..93391953 --- /dev/null +++ b/ornn-web/src/components/skill/TransferOwnershipModal.tsx @@ -0,0 +1,215 @@ +/** + * TransferOwnershipModal — hand a skill to another Ornn user (#1123). + * + * A Danger-Zone action: the owner picks another Ornn user (email typeahead + * against the directory, same source the PermissionsModal uses), then types + * the skill name to confirm. Transfer is immediate and irreversible by the + * caller — afterwards they're no longer the owner (they keep READ access), + * so the detail view refetches and the owner-only UI drops away. + * + * Single-target by design — distinct from the multi-select grant pickers. + * + * @module components/skill/TransferOwnershipModal + */ + +import { useEffect, useRef, useState } from "react"; +import { useTranslation } from "react-i18next"; +import { useQuery } from "@tanstack/react-query"; +import { Modal } from "@/components/ui/Modal"; +import { Button } from "@/components/ui/Button"; +import { useTransferSkillOwnership } from "@/hooks/useSkills"; +import { useToastStore } from "@/stores/toastStore"; +import { searchUsersByEmail, type UserDirectoryEntry } from "@/services/usersApi"; +import type { SkillDetail } from "@/types/domain"; +import { translateError } from "@/utils/translateError"; + +interface TransferOwnershipModalProps { + isOpen: boolean; + onClose: () => void; + skill: SkillDetail; +} + +function useDebouncedValue(value: T, delayMs: number): T { + const [debounced, setDebounced] = useState(value); + useEffect(() => { + const id = setTimeout(() => setDebounced(value), delayMs); + return () => clearTimeout(id); + }, [value, delayMs]); + return debounced; +} + +export function TransferOwnershipModal({ isOpen, onClose, skill }: TransferOwnershipModalProps) { + const { t } = useTranslation(); + return ( + + {/* Keyed on open + skill so the form resets by construction whenever + the modal reopens — no reset effect. */} + + + ); +} + +interface TransferFormProps { + skill: SkillDetail; + onClose: () => void; + t: ReturnType["t"]; +} + +function TransferForm({ skill, onClose, t }: TransferFormProps) { + const addToast = useToastStore((s) => s.addToast); + // Two-id split (#750): guid for the write; the broad list invalidation + // refreshes the active detail view regardless of how it was keyed. + const transferMutation = useTransferSkillOwnership(skill.guid, skill.guid); + + const [selected, setSelected] = useState(null); + const [query, setQuery] = useState(""); + const [focused, setFocused] = useState(false); + const [confirmName, setConfirmName] = useState(""); + const inputRef = useRef(null); + + const debounced = useDebouncedValue(query.trim(), 200); + // 2-char minimum mirrors the directory search guard (#816). + const shouldSearch = !selected && debounced.length >= 2; + const { data: suggestions = [] } = useQuery({ + queryKey: ["users-search", debounced], + queryFn: () => searchUsersByEmail(debounced, 8), + enabled: shouldSearch, + staleTime: 10_000, + }); + + const nameConfirmed = confirmName.trim() === skill.name; + const canTransfer = !!selected && nameConfirmed && !transferMutation.isPending; + + const handleTransfer = async () => { + if (!selected) return; + try { + await transferMutation.mutateAsync(selected.userId); + addToast({ + type: "success", + message: t("transfer.success", "Ownership transferred. You now have read access only."), + }); + onClose(); + } catch (err) { + addToast({ type: "error", message: translateError(err) }); + } + }; + + return ( + <> +

+ {t( + "transfer.explain", + "Transfer this skill to another Ornn user. They become the full owner immediately; you keep read access only and can no longer manage or delete it. This cannot be undone by you.", + )} +

+ + {/* ── Target picker (single select) ── */} +
+ + + {selected ? ( +
+ + {selected.displayName || selected.email || selected.userId} + + {selected.email && selected.email !== selected.displayName && ( + {selected.email} + )} + +
+ ) : ( +
+ setQuery(e.target.value)} + onFocus={() => setFocused(true)} + onBlur={() => setTimeout(() => setFocused(false), 150)} + placeholder={t("transfer.searchPlaceholder", "type an email to find a user...") as string} + className="w-full rounded border border-accent/20 bg-elevated px-3 py-2 font-text text-sm text-strong focus:border-accent/60 focus:outline-none" + /> + {focused && debounced.length < 2 && ( +
+

+ {t("transfer.searchHint", "Type at least 2 characters to search.")} +

+
+ )} + {focused && suggestions.length > 0 && ( +
+ {suggestions.map((s) => ( + + ))} +
+ )} +

+ {t("transfer.directoryNote", "Only users who have signed into Ornn appear here.")} +

+
+ )} +
+ + {/* ── Type-the-name confirm (only once a target is picked) ── */} + {selected && ( +
+ + setConfirmName(e.target.value)} + placeholder={skill.name} + autoComplete="off" + className="w-full rounded border border-danger/30 bg-elevated px-3 py-2 font-mono text-sm text-strong focus:border-danger/60 focus:outline-none" + /> +
+ )} + +
+ + +
+ + ); +} diff --git a/ornn-web/src/hooks/useSkillDetail.ts b/ornn-web/src/hooks/useSkillDetail.ts index 0dea1ef2..efd8616d 100644 --- a/ornn-web/src/hooks/useSkillDetail.ts +++ b/ornn-web/src/hooks/useSkillDetail.ts @@ -158,6 +158,7 @@ export function useSkillDetail(idOrName: string | undefined) { // ── Modal + file-edit state ───────────────────────────────── const [showDeleteConfirm, setShowDeleteConfirm] = useState(false); const [showPermissionsModal, setShowPermissionsModal] = useState(false); + const [showTransferModal, setShowTransferModal] = useState(false); const [showAdvancedModal, setShowAdvancedModal] = useState(false); const [showSaveConfirm, setShowSaveConfirm] = useState(false); const [showAuditStartedModal, setShowAuditStartedModal] = useState(false); @@ -423,6 +424,8 @@ export function useSkillDetail(idOrName: string | undefined) { setShowDeleteConfirm, showPermissionsModal, setShowPermissionsModal, + showTransferModal, + setShowTransferModal, showAdvancedModal, setShowAdvancedModal, showSaveConfirm, diff --git a/ornn-web/src/pages/skill/SkillDetailPage.tsx b/ornn-web/src/pages/skill/SkillDetailPage.tsx index 58eedca8..9b11bab2 100644 --- a/ornn-web/src/pages/skill/SkillDetailPage.tsx +++ b/ornn-web/src/pages/skill/SkillDetailPage.tsx @@ -33,6 +33,7 @@ import { AdvancedOptionsModal } from "@/components/skill/AdvancedOptionsModal"; import { VersionDiffModal } from "@/components/skill/VersionDiffModal"; import { SkillSaveConfirmModal } from "@/components/skill/SkillSaveConfirmModal"; import { SkillDeleteConfirmModal } from "@/components/skill/SkillDeleteConfirmModal"; +import { TransferOwnershipModal } from "@/components/skill/TransferOwnershipModal"; import { SkillAuditStartedModal } from "@/components/skill/SkillAuditStartedModal"; import { SkillVersionsBrowserModal } from "@/components/skill/SkillVersionsBrowserModal"; import { AuditVerdictPill } from "@/components/skill/AuditVerdictPill"; @@ -91,6 +92,8 @@ export function SkillDetailPage() { setShowDeleteConfirm, showPermissionsModal, setShowPermissionsModal, + showTransferModal, + setShowTransferModal, showAdvancedModal, setShowAdvancedModal, showSaveConfirm, @@ -519,6 +522,17 @@ export function SkillDetailPage() { {t("skillDetail.cardDanger", "Danger zone")} +

+ {t("skillDetail.transferExplain", "Transfer this skill to another Ornn user. They become the owner; you keep read access only.")} +

+

{t("skillDetail.dangerExplain", "Permanently delete this skill and every version. This cannot be undone.")}

@@ -596,6 +610,15 @@ export function SkillDetailPage() { /> )} + {/* ── Ownership transfer (danger zone) ── */} + {isOwner && ( + setShowTransferModal(false)} + skill={skill} + /> + )} + {/* ── Advanced options popup ── */} {isOwner && ( Date: Tue, 16 Jun 2026 02:08:01 +0800 Subject: [PATCH 13/17] feat(web): per-grant read / read-write level in the permissions editor (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The PermissionsModal now drives the typed `grants` ACL instead of bare id lists: - each granted user (chip) and checked org (row) carries a compact read / read-write toggle (`LevelToggle`); new grants default to read. - form state initializes per-grant levels from `skill.grants` (falling back to read-level from the legacy lists), the resolve-labels effect preserves the level, and Save builds + sends typed `grants` with level-aware change detection (a level flip alone now counts as a change). - unresolved (ghost) user grants hide the toggle — you can only revoke them. - `SkillVisibilityCard` gains a "N can edit" line counting read-write grants. A focused test covers init-from-grants, the read-write→read flip, and that Save sends the canonical typed-grants payload. Full web suite stays green. Part of #1123 --- .../skill/PermissionsModal.test.tsx | 126 ++++++++++++ .../src/components/skill/PermissionsModal.tsx | 185 ++++++++++++++---- .../components/skill/SkillVisibilityCard.tsx | 11 ++ ornn-web/src/pages/skill/SkillDetailPage.tsx | 1 + 4 files changed, 285 insertions(+), 38 deletions(-) create mode 100644 ornn-web/src/components/skill/PermissionsModal.test.tsx diff --git a/ornn-web/src/components/skill/PermissionsModal.test.tsx b/ornn-web/src/components/skill/PermissionsModal.test.tsx new file mode 100644 index 00000000..34bf2d45 --- /dev/null +++ b/ornn-web/src/components/skill/PermissionsModal.test.tsx @@ -0,0 +1,126 @@ +/** + * PermissionsModal level tests (#1123). + * + * Guards the typed-grant flow: an existing read_write user grant renders the + * read-write toggle, flipping it to read and saving sends the canonical + * `grants` payload with the new level. + * + * orgs / directory / mutation / toast are mocked; framer-motion is stubbed; + * react-i18next is global. + * + * @module components/skill/PermissionsModal.test + */ + +import { describe, expect, it, vi, beforeEach, afterEach } from "vitest"; +import { cleanup, fireEvent, render, screen, waitFor } from "@testing-library/react"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import type { SkillDetail } from "@/types/domain"; + +const mutateAsync = vi.fn(); +const addToast = vi.fn(); +const resolveUsers = vi.fn(); +const searchUsersByEmail = vi.fn(); + +vi.mock("framer-motion", () => ({ + AnimatePresence: ({ children }: { children: React.ReactNode }) => <>{children}, + motion: new Proxy( + {}, + { + get: (_t, tag: string) => { + const Comp: React.FC & { children?: React.ReactNode }> = ({ + children, + initial: _i, + animate: _a, + exit: _e, + transition: _tr, + ...rest + }) => { + void _i; void _a; void _e; void _tr; + const Tag = tag as keyof React.JSX.IntrinsicElements; + return {children}; + }; + return Comp; + }, + }, + ), +})); + +vi.mock("@/hooks/useMe", () => ({ useMyOrgs: () => ({ data: [] }) })); +vi.mock("@/hooks/useSkills", () => ({ + useUpdateSkillPermissions: () => ({ mutateAsync, isPending: false }), +})); +vi.mock("@/stores/toastStore", () => ({ + useToastStore: (selector: (s: { addToast: typeof addToast }) => T) => selector({ addToast }), +})); +vi.mock("@/services/usersApi", () => ({ + resolveUsers: (...a: unknown[]) => resolveUsers(...a), + searchUsersByEmail: (...a: unknown[]) => searchUsersByEmail(...a), + fetchOrgSummary: vi.fn(), +})); + +import { PermissionsModal } from "./PermissionsModal"; + +function skill(overrides: Partial = {}): SkillDetail { + return { + guid: "skill-guid", + name: "demo-skill", + description: "", + createdBy: "owner-1", + createdOn: "2026-05-01T00:00:00.000Z", + isPrivate: true, + tags: [], + updatedOn: "2026-05-01T00:00:00.000Z", + presignedPackageUrl: "", + metadata: {}, + version: "1.0", + sharedWithUsers: ["u1"], + sharedWithOrgs: [], + grants: [{ type: "user", id: "u1", level: "read_write" }], + ...overrides, + } as SkillDetail; +} + +function renderModal() { + const client = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + {}} skill={skill()} /> + , + ); +} + +beforeEach(() => { + mutateAsync.mockReset().mockResolvedValue({ skill: skill() }); + addToast.mockReset(); + // Resolve u1 to a real label so the chip is not "unresolved" → the level + // toggle renders. + resolveUsers.mockReset().mockResolvedValue([ + { userId: "u1", email: "u1@x.io", displayName: "User One" }, + ]); + searchUsersByEmail.mockReset().mockResolvedValue([]); +}); + +afterEach(() => cleanup()); + +describe("PermissionsModal — permission levels (#1123)", () => { + it("renders the read-write toggle for an existing read_write grant and saves the flipped level as typed grants", async () => { + renderModal(); + + // After the directory resolves u1, its chip shows the read-write toggle. + const toggle = await screen.findByRole("button", { name: /read-write/i }); + expect(toggle).toBeTruthy(); + + // Flip read-write → read. + fireEvent.click(toggle); + await screen.findByRole("button", { name: /^read$/i }); + + // Save sends the canonical typed grants with the new level. + fireEvent.click(screen.getByRole("button", { name: /^save$/i })); + await waitFor(() => + expect(mutateAsync).toHaveBeenCalledWith({ + isPrivate: true, + grants: [{ type: "user", id: "u1", level: "read" }], + }), + ); + }); +}); diff --git a/ornn-web/src/components/skill/PermissionsModal.tsx b/ornn-web/src/components/skill/PermissionsModal.tsx index 99d4af8f..91e308e1 100644 --- a/ornn-web/src/components/skill/PermissionsModal.tsx +++ b/ornn-web/src/components/skill/PermissionsModal.tsx @@ -31,9 +31,30 @@ import { fetchOrgSummary, type UserDirectoryEntry, } from "@/services/usersApi"; -import type { SkillDetail } from "@/types/domain"; +import type { SkillDetail, SkillGrant, SkillPermissionLevel } from "@/types/domain"; import { translateError } from "@/utils/translateError"; +/** A selected user grant carries its permission level alongside the label. */ +type UserGrantEntry = UserDirectoryEntry & { level: SkillPermissionLevel }; + +/** + * Resolve the skill's initial typed grants. Prefers the canonical `grants` + * field; falls back to deriving READ-level grants from the legacy lists for + * older cached payloads (#1123). + */ +function initialGrantsOf(skill: SkillDetail): SkillGrant[] { + if (skill.grants) return skill.grants; + return [ + ...skill.sharedWithUsers.map((id) => ({ type: "user" as const, id, level: "read" as const })), + ...skill.sharedWithOrgs.map((id) => ({ type: "org" as const, id, level: "read" as const })), + ]; +} + +/** Stable signature of a grant set for change-detection (order-independent). */ +function grantSignature(grants: SkillGrant[]): string { + return grants.map((g) => `${g.type}:${g.id}:${g.level}`).sort().join("|"); +} + interface PermissionsModalProps { isOpen: boolean; onClose: () => void; @@ -64,7 +85,7 @@ export function PermissionsModal({ isOpen, onClose, skill }: PermissionsModalPro cascading render (#888). The outer Modal owns the open/close animation, so its AnimatePresence stays stable. */} (!skill.isPrivate); - const [sharedUsers, setSharedUsers] = useState(() => - skill.sharedWithUsers.map((id) => ({ userId: id, email: "", displayName: id })), + const [sharedUsers, setSharedUsers] = useState(() => + initialGrants + .filter((g) => g.type === "user") + .map((g) => ({ userId: g.id, email: "", displayName: g.id, level: g.level })), + ); + const [sharedOrgIds, setSharedOrgIds] = useState(() => + initialGrants.filter((g) => g.type === "org").map((g) => g.id), + ); + // Per-org level, keyed by org id. Only checked orgs are read on save. + const [orgLevels, setOrgLevels] = useState>(() => + Object.fromEntries( + initialGrants.filter((g) => g.type === "org").map((g) => [g.id, g.level]), + ), ); - const [sharedOrgIds, setSharedOrgIds] = useState(skill.sharedWithOrgs); const [userQuery, setUserQuery] = useState(""); const [userInputFocused, setUserInputFocused] = useState(false); const userInputRef = useRef(null); @@ -109,7 +141,11 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { if (cancelled || resolved.length === 0) return; const byId = new Map(resolved.map((r) => [r.userId, r])); setSharedUsers((prev) => - prev.map((existing) => byId.get(existing.userId) ?? existing), + prev.map((existing) => { + const hit = byId.get(existing.userId); + // Merge the resolved label but PRESERVE the grant's level (#1123). + return hit ? { ...hit, level: existing.level } : existing; + }), ); })(); return () => { @@ -179,16 +215,27 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { setSharedOrgIds((prev) => prev.includes(orgId) ? prev.filter((id) => id !== orgId) : [...prev, orgId], ); + // New grants default to read; preserve an existing level on re-check. + setOrgLevels((prev) => (prev[orgId] ? prev : { ...prev, [orgId]: "read" })); + }; + + const setOrgLevel = (orgId: string, level: SkillPermissionLevel) => { + setOrgLevels((prev) => ({ ...prev, [orgId]: level })); }; const addUser = (entry: UserDirectoryEntry) => { if (sharedUsers.some((u) => u.userId === entry.userId)) return; - setSharedUsers((prev) => [...prev, entry]); + // New per-user grants default to read. + setSharedUsers((prev) => [...prev, { ...entry, level: "read" }]); setUserQuery(""); setUserInputFocused(false); userInputRef.current?.blur(); }; + const setUserLevel = (userId: string, level: SkillPermissionLevel) => { + setSharedUsers((prev) => prev.map((u) => (u.userId === userId ? { ...u, level } : u))); + }; + const removeUser = (userId: string) => { setSharedUsers((prev) => prev.filter((u) => u.userId !== userId)); }; @@ -197,22 +244,26 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { const usersActive = !isPublic && sharedUsers.length > 0; const privateActive = !isPublic && !orgsActive && !usersActive; + // The canonical typed grants this form would persist (#1123). + const buildGrants = (): SkillGrant[] => [ + ...sharedUsers.map((u) => ({ type: "user" as const, id: u.userId, level: u.level })), + ...sharedOrgIds.map((id) => ({ + type: "org" as const, + id, + level: orgLevels[id] ?? "read", + })), + ]; + const handleSave = async () => { - // Quick client-side "nothing changed" short-circuit. - const beforePrivate = skill.isPrivate; - const beforeUsers = new Set(skill.sharedWithUsers); - const beforeOrgs = new Set(skill.sharedWithOrgs); const afterPrivate = !isPublic; + const grants = buildGrants(); - const privateChanged = beforePrivate !== afterPrivate; - const usersChanged = - sharedUsers.length !== beforeUsers.size || - sharedUsers.some((u) => !beforeUsers.has(u.userId)); - const orgsChanged = - sharedOrgIds.length !== beforeOrgs.size || - sharedOrgIds.some((id) => !beforeOrgs.has(id)); + // Level-aware "nothing changed" short-circuit (covers a level flip, + // not just add/remove). + const privateChanged = skill.isPrivate !== afterPrivate; + const grantsChanged = grantSignature(grants) !== grantSignature(initialGrants); - if (!privateChanged && !usersChanged && !orgsChanged) { + if (!privateChanged && !grantsChanged) { addToast({ type: "info", message: t("permissions.noChanges", "No changes to save."), @@ -223,9 +274,8 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { try { await permissionsMutation.mutateAsync({ - isPrivate: !isPublic, - sharedWithUsers: sharedUsers.map((u) => u.userId), - sharedWithOrgs: sharedOrgIds, + isPrivate: afterPrivate, + grants, }); addToast({ type: "success", @@ -339,19 +389,28 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { {org.displayName} - {org.isUnresolved ? ( - - - - - - {t("permissions.orgUnresolved", "unresolved")} - - ) : !org.isMember ? ( - - {t("permissions.notMember", "not member")} - - ) : null} + + {checked && ( + setOrgLevel(org.userId, lvl)} + t={t} + /> + )} + {org.isUnresolved ? ( + + + + + + {t("permissions.orgUnresolved", "unresolved")} + + ) : !org.isMember ? ( + + {t("permissions.notMember", "not member")} + + ) : null} + ); })} @@ -410,6 +469,13 @@ function PermissionsForm({ skill, onClose, t }: PermissionsFormProps) { )} {u.email || u.displayName || u.userId} + {!isUnresolved && ( + setUserLevel(u.userId, lvl)} + t={t} + /> + )} + ); +} + /** Section divider with a small uppercase label centered on top. */ function SectionHeader({ label }: { label: string }) { return ( diff --git a/ornn-web/src/components/skill/SkillVisibilityCard.tsx b/ornn-web/src/components/skill/SkillVisibilityCard.tsx index 68223cd2..3731d492 100644 --- a/ornn-web/src/components/skill/SkillVisibilityCard.tsx +++ b/ornn-web/src/components/skill/SkillVisibilityCard.tsx @@ -24,6 +24,8 @@ export interface SkillVisibilityCardProps { isPrivate: boolean; sharedWithUsersCount: number; sharedWithOrgsCount: number; + /** How many grants confer read-write (#1123). Shown as a "can edit" line. */ + readWriteCount?: number; isOwner: boolean; onManagePermissions: () => void; } @@ -32,6 +34,7 @@ export function SkillVisibilityCard({ isPrivate, sharedWithUsersCount, sharedWithOrgsCount, + readWriteCount = 0, isOwner, onManagePermissions, }: SkillVisibilityCardProps) { @@ -97,6 +100,14 @@ export function SkillVisibilityCard({ {t("skillDetail.shareOrgs", "organizations")} + {readWriteCount > 0 && ( +
  • + + {readWriteCount} + + {t("skillDetail.shareCanEdit", "can edit")} +
    • + )} {isOwner && (
    diff --git a/ornn-web/src/pages/skill/SkillDetailPage.tsx b/ornn-web/src/pages/skill/SkillDetailPage.tsx index 9b11bab2..cce64767 100644 --- a/ornn-web/src/pages/skill/SkillDetailPage.tsx +++ b/ornn-web/src/pages/skill/SkillDetailPage.tsx @@ -367,6 +367,7 @@ export function SkillDetailPage() { isPrivate={skill.isPrivate} sharedWithUsersCount={skill.sharedWithUsers.length} sharedWithOrgsCount={skill.sharedWithOrgs.length} + readWriteCount={(skill.grants ?? []).filter((g) => g.level === "read_write").length} isOwner={isOwner} onManagePermissions={() => setShowPermissionsModal(true)} /> From 333f12c584ef8b6dd7858223275dd43b66e6a9c9 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 02:08:38 +0800 Subject: [PATCH 14/17] =?UTF-8?q?feat(sdk):=20TypeScript=20SDK=20=E2=80=94?= =?UTF-8?q?=20typed=20grants=20+=20ownership=20transfer=20(#1123)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Expose the new permission surface in @chronoai/ornn-sdk: - `SkillPermissionLevel` + `SkillGrant` types; `grants` on SkillDetail / SkillsetDetail; a shared `PermissionsInput` (carries `grants`, legacy `sharedWith*` still accepted) re-exported as the skill/skillset aliases. - `setSkillPermissions(id, …)` — the SDK had no skill permissions setter before (only skillsets); this fills the gap. - `transferSkillOwnership(id, newOwnerUserId)` and `transferSkillsetOwnership(id, newOwnerUserId)`. - `setSkillsetPermissions` now accepts `grants`. Transport plumbing (envelope/error/zip helpers) extracted into a new `http.ts` so `client.ts` stays under the 500-line cap after the additions. 8 new tests (transfer success + 403, grants setters + 400 validation); 38 pass; tsc + lint clean. Part of #1123 --- sdk/typescript/src/client.ts | 130 +++++++++++--------- sdk/typescript/src/http.ts | 68 +++++++++++ sdk/typescript/src/index.ts | 4 + sdk/typescript/src/tests/client.test.ts | 154 ++++++++++++++++++++++++ sdk/typescript/src/types.ts | 45 ++++++- 5 files changed, 343 insertions(+), 58 deletions(-) create mode 100644 sdk/typescript/src/http.ts diff --git a/sdk/typescript/src/client.ts b/sdk/typescript/src/client.ts index fc2588ba..c57ee71b 100644 --- a/sdk/typescript/src/client.ts +++ b/sdk/typescript/src/client.ts @@ -15,7 +15,14 @@ * @module client */ -import { OrnnError, type OrnnErrorPayload } from "./errors"; +import { OrnnError } from "./errors"; +import { + buildError, + parseError, + zipToBlob, + type EnvelopeSuccess, + type ProblemJson, +} from "./http"; import type { ClosureNode, ClosureResult, @@ -23,6 +30,7 @@ import type { PublishOptions, PublishSkillsetInput, SkillDetail, + SkillPermissionsInput, SkillSearchParams, SkillSearchResult, SkillsetClosureResult, @@ -45,27 +53,6 @@ export interface OrnnClientOptions { readonly fetch?: typeof fetch; } -interface EnvelopeSuccess { - readonly data: T; - readonly error: null; -} - -/** - * Wire shape for error responses post-#456 — RFC 7807 - * `application/problem+json`. Fields at the body root; `error` / - * `data` are gone. - */ -interface ProblemJson { - readonly type?: string; - readonly title?: string; - readonly status?: number; - readonly code?: string; - readonly detail?: string; - readonly instance?: string; - readonly requestId?: string; - readonly errors?: ReadonlyArray<{ path?: string; code?: string; message: string }>; -} - export class OrnnClient { private readonly baseUrl: string; private readonly staticToken: string | undefined; @@ -284,6 +271,45 @@ export class OrnnClient { await this.request<{ success: boolean }>("DELETE", `/skills/${encodeURIComponent(id)}`); } + /** + * Update a skill's visibility / sharing ACL (#1123). `grants` is the + * canonical typed ACL — each entry grants a user or org a `read` or + * `read_write` level. The legacy `sharedWith*` arrays are still accepted + * (mapped to `read`-level grants) for backward compat. Returns the + * refreshed skill detail. + */ + async setSkillPermissions( + id: string, + input: SkillPermissionsInput, + ): Promise { + const res = await this.request<{ skill: SkillDetail }>( + "PUT", + `/skills/${encodeURIComponent(id)}/permissions`, + { + body: JSON.stringify(input), + headers: { "Content-Type": "application/json" }, + }, + ); + return res.skill; + } + + /** + * Transfer ownership of a skill to another user (#1123). The new owner + * becomes `createdBy`; the previous owner keeps no implicit access. + * Returns the refreshed skill detail. + */ + async transferSkillOwnership(id: string, newOwnerUserId: string): Promise { + const res = await this.request<{ skill: SkillDetail }>( + "POST", + `/skills/${encodeURIComponent(id)}/transfer-ownership`, + { + body: JSON.stringify({ newOwnerUserId }), + headers: { "Content-Type": "application/json" }, + }, + ); + return res.skill; + } + // ---- Skillsets (#969) ---- /** @@ -316,7 +342,12 @@ export class OrnnClient { }); } - /** Update a skillset's visibility / sharing lists. */ + /** + * Update a skillset's visibility / sharing ACL (#1123). `grants` is the + * canonical typed ACL — each entry grants a user or org a `read` or + * `read_write` level. The legacy `sharedWith*` arrays are still accepted + * (mapped to `read`-level grants) for backward compat. + */ async setSkillsetPermissions( id: string, input: SkillsetPermissionsInput, @@ -332,6 +363,26 @@ export class OrnnClient { return res.skillset; } + /** + * Transfer ownership of a skillset to another user (#1123). The new + * owner becomes `createdBy`; the previous owner keeps no implicit + * access. Returns the refreshed skillset detail. + */ + async transferSkillsetOwnership( + id: string, + newOwnerUserId: string, + ): Promise { + const res = await this.request<{ skillset: SkillsetDetail }>( + "POST", + `/skillsets/${encodeURIComponent(id)}/transfer-ownership`, + { + body: JSON.stringify({ newOwnerUserId }), + headers: { "Content-Type": "application/json" }, + }, + ); + return res.skillset; + } + /** Delete a skillset (and all its versions) by ID. */ async deleteSkillset(id: string): Promise { await this.request<{ success: boolean }>( @@ -427,36 +478,3 @@ export class OrnnClient { }); } } - -function zipToBlob(zip: Blob | ArrayBuffer | Uint8Array): Blob { - if (zip instanceof Blob) return zip; - return new Blob([zip as BlobPart], { type: "application/zip" }); -} - -async function parseError(res: Response): Promise { - const body = (await res.json().catch(() => null)) as ProblemJson | null; - return buildError(res.status, body); -} - -function buildError(status: number, body: ProblemJson | null): OrnnError { - if (body && (body.code || body.detail || body.title)) { - // exactOptionalPropertyTypes (#450): only stamp `requestId` / - // `errors` keys when the upstream actually provided them — the - // payload type is `requestId?: string`, not `string | undefined`, - // so `{ requestId: undefined }` is a type error under the - // stricter contract. - const payload: OrnnErrorPayload = { - status: body.status ?? status, - code: body.code ?? "unknown_error", - message: body.detail ?? body.title ?? `Ornn API returned ${status}`, - ...(body.requestId !== undefined ? { requestId: body.requestId } : {}), - ...(body.errors !== undefined ? { errors: body.errors } : {}), - }; - return new OrnnError(payload); - } - return new OrnnError({ - status, - code: "unknown_error", - message: `Ornn API returned ${status} without a recognized error body`, - }); -} diff --git a/sdk/typescript/src/http.ts b/sdk/typescript/src/http.ts new file mode 100644 index 00000000..05c5ab1f --- /dev/null +++ b/sdk/typescript/src/http.ts @@ -0,0 +1,68 @@ +/** + * HTTP plumbing for the Ornn client — response-envelope shapes and + * error mapping, kept separate from {@link OrnnClient}'s endpoint + * methods so the transport machinery stays small and stable while the + * business-logic surface (the methods) grows. + * + * @module http + */ + +import { OrnnError, type OrnnErrorPayload } from "./errors"; + +/** Success envelope per CONVENTIONS.md — `{ data, error: null }`. */ +export interface EnvelopeSuccess { + readonly data: T; + readonly error: null; +} + +/** + * Wire shape for error responses post-#456 — RFC 7807 + * `application/problem+json`. Fields at the body root; `error` / + * `data` are gone. + */ +export interface ProblemJson { + readonly type?: string; + readonly title?: string; + readonly status?: number; + readonly code?: string; + readonly detail?: string; + readonly instance?: string; + readonly requestId?: string; + readonly errors?: ReadonlyArray<{ path?: string; code?: string; message: string }>; +} + +/** Coerce any accepted package representation into a ZIP-typed Blob. */ +export function zipToBlob(zip: Blob | ArrayBuffer | Uint8Array): Blob { + if (zip instanceof Blob) return zip; + return new Blob([zip as BlobPart], { type: "application/zip" }); +} + +/** Read a non-2xx response body and map it to an {@link OrnnError}. */ +export async function parseError(res: Response): Promise { + const body = (await res.json().catch(() => null)) as ProblemJson | null; + return buildError(res.status, body); +} + +/** Build an {@link OrnnError} from an RFC 7807 problem body (or a bare status). */ +export function buildError(status: number, body: ProblemJson | null): OrnnError { + if (body && (body.code || body.detail || body.title)) { + // exactOptionalPropertyTypes (#450): only stamp `requestId` / + // `errors` keys when the upstream actually provided them — the + // payload type is `requestId?: string`, not `string | undefined`, + // so `{ requestId: undefined }` is a type error under the + // stricter contract. + const payload: OrnnErrorPayload = { + status: body.status ?? status, + code: body.code ?? "unknown_error", + message: body.detail ?? body.title ?? `Ornn API returned ${status}`, + ...(body.requestId !== undefined ? { requestId: body.requestId } : {}), + ...(body.errors !== undefined ? { errors: body.errors } : {}), + }; + return new OrnnError(payload); + } + return new OrnnError({ + status, + code: "unknown_error", + message: `Ornn API returned ${status} without a recognized error body`, + }); +} diff --git a/sdk/typescript/src/index.ts b/sdk/typescript/src/index.ts index 8065f1b2..96ba9b25 100644 --- a/sdk/typescript/src/index.ts +++ b/sdk/typescript/src/index.ts @@ -25,10 +25,14 @@ export type { ClosureNode, ClosureResult, CreateSkillsetInput, + PermissionsInput, PublishOptions, PublishSkillsetInput, SearchScope, SkillDetail, + SkillGrant, + SkillPermissionLevel, + SkillPermissionsInput, SkillSearchParams, SkillSearchResult, SkillsetClosureResult, diff --git a/sdk/typescript/src/tests/client.test.ts b/sdk/typescript/src/tests/client.test.ts index 1f57f416..861c2f6e 100644 --- a/sdk/typescript/src/tests/client.test.ts +++ b/sdk/typescript/src/tests/client.test.ts @@ -364,6 +364,99 @@ describe("OrnnClient", () => { expect(capturedMethod).toBe("DELETE"); expect(capturedUrl).toBe("https://x/api/v1/skills/abc"); }); + + // -- Permissions + ownership (#1123) -- + + test("setSkillPermissions(): PUTs grants ACL and unwraps the { skill } envelope", async () => { + let captured = { method: "", url: "", body: "", ct: "" }; + const fetchMock = mockFetch((url, init) => { + captured = { + method: init.method ?? "", + url, + body: init.body as string, + ct: (init.headers as Record)["Content-Type"] ?? "", + }; + return jsonResponse(200, { + data: { + skill: { + id: "sk-1", + isPrivate: false, + grants: [{ type: "user", id: "u-2", level: "read_write" }], + }, + }, + error: null, + }); + }); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const res = await client.setSkillPermissions("sk-1", { + isPrivate: false, + grants: [{ type: "user", id: "u-2", level: "read_write" }], + }); + expect(captured.method).toBe("PUT"); + expect(captured.url).toBe("https://x/api/v1/skills/sk-1/permissions"); + expect(captured.ct).toBe("application/json"); + // The typed grant shape (#1123) is sent verbatim on the wire. + expect(JSON.parse(captured.body)).toEqual({ + isPrivate: false, + grants: [{ type: "user", id: "u-2", level: "read_write" }], + }); + expect(res.id).toBe("sk-1"); + expect(res.grants).toEqual([{ type: "user", id: "u-2", level: "read_write" }]); + }); + + test("setSkillPermissions(): throws OrnnError on validation failure (400)", async () => { + const fetchMock = mockFetch(() => + jsonResponse(400, { + status: 400, + code: "validation_error", + detail: "grants[0].level must be read|read_write", + }), + ); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const err = (await client + .setSkillPermissions("sk-1", { + isPrivate: true, + grants: [{ type: "user", id: "u-2", level: "read" }], + }) + .catch((e) => e)) as OrnnError; + expect(err).toBeInstanceOf(OrnnError); + expect(err.status).toBe(400); + expect(err.code).toBe("validation_error"); + }); + + test("transferSkillOwnership(): POSTs newOwnerUserId and unwraps { skill }", async () => { + let captured = { method: "", url: "", body: "" }; + const fetchMock = mockFetch((url, init) => { + captured = { method: init.method ?? "", url, body: init.body as string }; + return jsonResponse(200, { + data: { skill: { id: "sk-1", createdBy: "u-2" } }, + error: null, + }); + }); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const res = await client.transferSkillOwnership("sk-1", "u-2"); + expect(captured.method).toBe("POST"); + expect(captured.url).toBe("https://x/api/v1/skills/sk-1/transfer-ownership"); + expect(JSON.parse(captured.body)).toEqual({ newOwnerUserId: "u-2" }); + expect(res.createdBy).toBe("u-2"); + }); + + test("transferSkillOwnership(): throws OrnnError on 403", async () => { + const fetchMock = mockFetch(() => + jsonResponse(403, { + status: 403, + code: "permission_denied", + detail: "only the owner can transfer", + }), + ); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const err = (await client + .transferSkillOwnership("sk-1", "u-2") + .catch((e) => e)) as OrnnError; + expect(err).toBeInstanceOf(OrnnError); + expect(err.status).toBe(403); + expect(err.code).toBe("permission_denied"); + }); }); // ====================================================================== @@ -453,6 +546,67 @@ describe("OrnnClient — skillsets", () => { expect(res.isPrivate).toBe(false); }); + test("setSkillsetPermissions(): sends the typed grants ACL on the wire (#1123)", async () => { + let captured = { url: "", body: "" }; + const fetchMock = mockFetch((url, init) => { + captured = { url, body: init.body as string }; + return jsonResponse(200, { + data: { + skillset: { + guid: "ss-1", + isPrivate: false, + grants: [{ type: "org", id: "o-9", level: "read" }], + }, + }, + error: null, + }); + }); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const res = await client.setSkillsetPermissions("ss-1", { + isPrivate: false, + grants: [{ type: "org", id: "o-9", level: "read" }], + }); + expect(captured.url).toBe("https://x/api/v1/skillsets/ss-1/permissions"); + expect(JSON.parse(captured.body).grants).toEqual([ + { type: "org", id: "o-9", level: "read" }, + ]); + expect(res.grants).toEqual([{ type: "org", id: "o-9", level: "read" }]); + }); + + test("transferSkillsetOwnership(): POSTs newOwnerUserId and unwraps { skillset }", async () => { + let captured = { method: "", url: "", body: "" }; + const fetchMock = mockFetch((url, init) => { + captured = { method: init.method ?? "", url, body: init.body as string }; + return jsonResponse(200, { + data: { skillset: { guid: "ss-1", createdBy: "u-2" } }, + error: null, + }); + }); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const res = await client.transferSkillsetOwnership("ss-1", "u-2"); + expect(captured.method).toBe("POST"); + expect(captured.url).toBe("https://x/api/v1/skillsets/ss-1/transfer-ownership"); + expect(JSON.parse(captured.body)).toEqual({ newOwnerUserId: "u-2" }); + expect(res.createdBy).toBe("u-2"); + }); + + test("transferSkillsetOwnership(): throws OrnnError on 403", async () => { + const fetchMock = mockFetch(() => + jsonResponse(403, { + status: 403, + code: "permission_denied", + detail: "only the owner can transfer", + }), + ); + const client = new OrnnClient({ baseUrl: "https://x", fetch: fetchMock }); + const err = (await client + .transferSkillsetOwnership("ss-1", "u-2") + .catch((e) => e)) as OrnnError; + expect(err).toBeInstanceOf(OrnnError); + expect(err.status).toBe(403); + expect(err.code).toBe("permission_denied"); + }); + test("deleteSkillset(): fires DELETE to /skillsets/:id", async () => { let captured = { method: "", url: "" }; const fetchMock = mockFetch((url, init) => { diff --git a/sdk/typescript/src/types.ts b/sdk/typescript/src/types.ts index 242f7f6a..0f0394bf 100644 --- a/sdk/typescript/src/types.ts +++ b/sdk/typescript/src/types.ts @@ -11,6 +11,27 @@ export type Visibility = "public" | "private"; export type SearchScope = "public" | "private" | "mine" | "mixed" | "shared-with-me"; +/** + * Access level a grant confers (#1123). `read` allows pull/execute; + * `read_write` additionally allows publishing new versions / editing. + */ +export type SkillPermissionLevel = "read" | "read_write"; + +/** + * One typed ACL entry (#1123) — the canonical sharing primitive for + * skills and skillsets. Grants a single user or org a specific access + * level. Supersedes the legacy `sharedWith*` arrays (which the server + * still accepts, mapping each entry to a `read`-level grant). + */ +export interface SkillGrant { + /** Whether the grant targets an individual user or a whole org. */ + readonly type: "user" | "org"; + /** The user or org id the grant applies to. */ + readonly id: string; + /** The access level conferred. */ + readonly level: SkillPermissionLevel; +} + /** Minimal skill summary as returned by search results. */ export interface SkillSummary { readonly id: string; @@ -30,6 +51,8 @@ export interface SkillDetail extends SkillSummary { readonly storageKey?: string; readonly sharedWithUsers?: readonly string[]; readonly sharedWithOrgs?: readonly string[]; + /** Canonical typed ACL (#1123). Present on detail reads post-#1123. */ + readonly grants?: readonly SkillGrant[]; } export interface SkillVersionEntry { @@ -147,6 +170,8 @@ export interface SkillsetDetail { readonly createdBy: string; readonly sharedWithUsers?: readonly string[]; readonly sharedWithOrgs?: readonly string[]; + /** Canonical typed ACL (#1123). Present on detail reads post-#1123. */ + readonly grants?: readonly SkillGrant[]; readonly createdOn: string; readonly updatedOn: string; } @@ -214,13 +239,29 @@ export interface SkillsetClosureResult { readonly items: readonly ClosureNode[]; } -/** Payload for `client.setSkillsetPermissions(id, ...)`. */ -export interface SkillsetPermissionsInput { +/** + * Payload for the permissions setters (#1123). Shared by skills and + * skillsets — `PUT /skills/:id/permissions` and + * `PUT /skillsets/:id/permissions` take the identical body. + * + * `grants` is the canonical ACL. The legacy `sharedWith*` arrays are + * still accepted by the server and map to `read`-level grants; prefer + * `grants` for new code. + */ +export interface PermissionsInput { readonly isPrivate: boolean; + /** Canonical typed ACL (#1123) — preferred over the `sharedWith*` arrays. */ + readonly grants?: readonly SkillGrant[]; readonly sharedWithUsers?: readonly string[]; readonly sharedWithOrgs?: readonly string[]; } +/** Payload for `client.setSkillPermissions(id, ...)` (#1123). */ +export type SkillPermissionsInput = PermissionsInput; + +/** Payload for `client.setSkillsetPermissions(id, ...)`. */ +export type SkillsetPermissionsInput = PermissionsInput; + export interface SkillsetSearchParams { /** Filter by kind (e.g. `consensus-supported`). */ readonly kind?: SkillsetKind; From 97add7fb4b075969ac9375bfd417a970b90f689d Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 02:09:10 +0800 Subject: [PATCH 15/17] =?UTF-8?q?feat(sdk):=20Python=20SDK=20=E2=80=94=20t?= =?UTF-8?q?yped=20grants=20+=20ownership=20transfer=20(#1123)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirror the TypeScript SDK additions in the Python client: - `SkillPermissionLevel` literal + `SkillGrant` dataclass (with from_dict/to_json); optional `grants` parsed onto SkillDetail / SkillsetDetail (defaults to [] on pre-#1123 responses). - `set_skill_permissions(...)` — NEW (no skill permissions setter existed before); `set_skillset_permissions(...)` gains a `grants` kwarg; a shared `_permissions_payload` builds both bodies and omits `grants` from the wire when not supplied (so the server's legacy fallback holds). - `transfer_skill_ownership(id, new_owner_user_id)` and `transfer_skillset_ownership(id, new_owner_user_id)`. 45 pytest pass (py3.9 + py3.12); ruff + strict mypy clean. Part of #1123 --- sdk/python/src/ornn_sdk/__init__.py | 4 + sdk/python/src/ornn_sdk/client.py | 109 +++++++++++++++- sdk/python/src/ornn_sdk/types.py | 44 ++++++- sdk/python/tests/test_client.py | 190 ++++++++++++++++++++++++++++ 4 files changed, 340 insertions(+), 7 deletions(-) diff --git a/sdk/python/src/ornn_sdk/__init__.py b/sdk/python/src/ornn_sdk/__init__.py index e1c99e49..647b4e98 100644 --- a/sdk/python/src/ornn_sdk/__init__.py +++ b/sdk/python/src/ornn_sdk/__init__.py @@ -26,6 +26,8 @@ SearchMode, SearchScope, SkillDetail, + SkillGrant, + SkillPermissionLevel, SkillSearchResult, SkillsetClosureResult, SkillsetDetail, @@ -47,6 +49,8 @@ "SearchMode", "SearchScope", "SkillDetail", + "SkillGrant", + "SkillPermissionLevel", "SkillSearchResult", "SkillsetClosureResult", "SkillsetDetail", diff --git a/sdk/python/src/ornn_sdk/client.py b/sdk/python/src/ornn_sdk/client.py index 55b6f649..126fe81b 100644 --- a/sdk/python/src/ornn_sdk/client.py +++ b/sdk/python/src/ornn_sdk/client.py @@ -27,6 +27,7 @@ SearchMode, SearchScope, SkillDetail, + SkillGrant, SkillSearchResult, SkillsetClosureResult, SkillsetDetail, @@ -239,6 +240,50 @@ def delete(self, skill_id: str) -> None: """Delete a skill by ID.""" self.request("DELETE", f"/skills/{_quote(skill_id)}") + def set_skill_permissions( + self, + skill_id: str, + *, + is_private: bool, + grants: list[SkillGrant] | None = None, + shared_with_users: list[str] | None = None, + shared_with_orgs: list[str] | None = None, + ) -> SkillDetail: + """Apply a new ACL state to a skill (#1123). + + ``grants`` is the canonical typed ACL — each entry confers ``read`` + or ``read_write`` to one user or org. The legacy ``shared_with_*`` + lists are still accepted (the server maps each entry to a + ``read``-level grant) but are superseded by ``grants``; pass one or + the other, not both. ``is_private=False`` makes the skill fully + public — grants are still persisted so the author can flip back to + private without losing their collaborator list. + """ + payload = _permissions_payload( + is_private=is_private, + grants=grants, + shared_with_users=shared_with_users, + shared_with_orgs=shared_with_orgs, + ) + data = self.request("PUT", f"/skills/{_quote(skill_id)}/permissions", json=payload) + return SkillDetail.from_dict(data["skill"]) + + def transfer_skill_ownership(self, skill_id: str, new_owner_user_id: str) -> SkillDetail: + """Hand a skill to another Ornn user (#1123). + + ADMIN-tier: only the skill's author or a platform admin may transfer. + The target must be a known Ornn user (signed in at least once) or the + call raises :class:`OrnnError` with code ``invalid_transfer_target``. + Transfer is immediate — the new owner becomes the owner synchronously + and the prior owner is kept as a ``read`` grantee. + """ + data = self.request( + "POST", + f"/skills/{_quote(skill_id)}/transfer-ownership", + json={"newOwnerUserId": new_owner_user_id}, + ) + return SkillDetail.from_dict(data["skill"]) + # ---- Skillsets (#969) --------------------------------------------------- def create_skillset( @@ -320,18 +365,46 @@ def set_skillset_permissions( skillset_id: str, *, is_private: bool, + grants: list[SkillGrant] | None = None, shared_with_users: list[str] | None = None, shared_with_orgs: list[str] | None = None, ) -> SkillsetDetail: - """Update a skillset's visibility / sharing lists.""" - payload: dict[str, Any] = { - "isPrivate": is_private, - "sharedWithUsers": shared_with_users or [], - "sharedWithOrgs": shared_with_orgs or [], - } + """Update a skillset's visibility / ACL (#1123). + + ``grants`` is the canonical typed ACL — each entry confers ``read`` + or ``read_write`` to one user or org. The legacy ``shared_with_*`` + lists are still accepted (the server maps each entry to a + ``read``-level grant) but are superseded by ``grants``; pass one or + the other, not both. + """ + payload = _permissions_payload( + is_private=is_private, + grants=grants, + shared_with_users=shared_with_users, + shared_with_orgs=shared_with_orgs, + ) data = self.request("PUT", f"/skillsets/{_quote(skillset_id)}/permissions", json=payload) return SkillsetDetail.from_dict(data["skillset"]) + def transfer_skillset_ownership( + self, skillset_id: str, new_owner_user_id: str + ) -> SkillsetDetail: + """Hand a skillset to another Ornn user (#1123). + + ADMIN-tier: only the skillset's author or a platform admin may + transfer. The target must be a known Ornn user (signed in at least + once) or the call raises :class:`OrnnError` with code + ``invalid_transfer_target``. Transfer is immediate — the new owner + becomes the owner synchronously and the prior owner is kept as a + ``read`` grantee. + """ + data = self.request( + "POST", + f"/skillsets/{_quote(skillset_id)}/transfer-ownership", + json={"newOwnerUserId": new_owner_user_id}, + ) + return SkillsetDetail.from_dict(data["skillset"]) + def delete_skillset(self, skillset_id: str) -> None: """Delete a skillset (and all its versions) by ID.""" self.request("DELETE", f"/skillsets/{_quote(skillset_id)}") @@ -422,6 +495,30 @@ def _quote(segment: str) -> str: return quote(segment, safe="") +def _permissions_payload( + *, + is_private: bool, + grants: list[SkillGrant] | None, + shared_with_users: list[str] | None, + shared_with_orgs: list[str] | None, +) -> dict[str, Any]: + """Build the shared body for the skill / skillset permissions endpoints (#1123). + + ``grants`` is the canonical typed ACL and is omitted from the wire when + not supplied so the server falls back to the legacy ``sharedWith*`` + lists (mapping each entry to a ``read``-level grant) for pre-#1123 + callers. + """ + payload: dict[str, Any] = { + "isPrivate": is_private, + "sharedWithUsers": shared_with_users or [], + "sharedWithOrgs": shared_with_orgs or [], + } + if grants is not None: + payload["grants"] = [g.to_json() for g in grants] + return payload + + def _build_error(res: httpx.Response, body: Any | None = None) -> OrnnError: """Build an OrnnError from an RFC 7807 problem+json response (#456). diff --git a/sdk/python/src/ornn_sdk/types.py b/sdk/python/src/ornn_sdk/types.py index ad5e2ac5..bff31eef 100644 --- a/sdk/python/src/ornn_sdk/types.py +++ b/sdk/python/src/ornn_sdk/types.py @@ -20,6 +20,40 @@ SystemFilter = Literal["any", "only", "exclude"] SkillsetKind = Literal["generic", "consensus-supported"] +# Access level a grant confers (#1123). `read` allows pull/execute; +# `read_write` additionally allows publishing new versions / editing. +SkillPermissionLevel = Literal["read", "read_write"] +GrantTarget = Literal["user", "org"] + + +@dataclass +class SkillGrant: + """One typed ACL entry (#1123) — the canonical sharing primitive for + skills and skillsets. + + Grants a single user or org a specific access level. Supersedes the + legacy ``sharedWith*`` arrays (which the server still accepts, mapping + each entry to a ``read``-level grant). + """ + + # Whether the grant targets an individual user or a whole org. + type: GrantTarget + # The user or org id the grant applies to. + id: str + # The access level conferred. + level: SkillPermissionLevel + + @classmethod + def from_dict(cls, raw: dict[str, Any]) -> SkillGrant: + return cls( + type=raw["type"], + id=raw["id"], + level=raw.get("level", "read"), + ) + + def to_json(self) -> dict[str, Any]: + return {"type": self.type, "id": self.id, "level": self.level} + @dataclass class SkillSummary: @@ -70,6 +104,9 @@ class SkillDetail(SkillSummary): storage_key: str | None = None shared_with_users: list[str] = field(default_factory=list) shared_with_orgs: list[str] = field(default_factory=list) + # Canonical typed ACL (#1123). Present on detail reads post-#1123; + # absent on pre-#1123 API responses (left as an empty list). + grants: list[SkillGrant] = field(default_factory=list) @classmethod def from_dict(cls, raw: dict[str, Any]) -> SkillDetail: @@ -77,7 +114,7 @@ def from_dict(cls, raw: dict[str, Any]) -> SkillDetail: base.pop("_extra") # `ownerId` was removed from the wire in #581 — tolerate it # showing up on old API responses (treat as unknown extra). - known_extra = {"storageKey", "sharedWithUsers", "sharedWithOrgs"} + known_extra = {"storageKey", "sharedWithUsers", "sharedWithOrgs", "grants"} summary_known = { "id", "name", @@ -100,6 +137,7 @@ def from_dict(cls, raw: dict[str, Any]) -> SkillDetail: storage_key=raw.get("storageKey"), shared_with_users=list(raw.get("sharedWithUsers") or []), shared_with_orgs=list(raw.get("sharedWithOrgs") or []), + grants=[SkillGrant.from_dict(g) for g in raw.get("grants") or []], _extra=extra, ) @@ -234,6 +272,9 @@ class SkillsetDetail: members: list[str] = field(default_factory=list) shared_with_users: list[str] = field(default_factory=list) shared_with_orgs: list[str] = field(default_factory=list) + # Canonical typed ACL (#1123). Present on detail reads post-#1123; + # absent on pre-#1123 API responses (left as an empty list). + grants: list[SkillGrant] = field(default_factory=list) created_on: str = "" updated_on: str = "" @@ -253,6 +294,7 @@ def from_dict(cls, raw: dict[str, Any]) -> SkillsetDetail: members=list(raw.get("members") or []), shared_with_users=list(raw.get("sharedWithUsers") or []), shared_with_orgs=list(raw.get("sharedWithOrgs") or []), + grants=[SkillGrant.from_dict(g) for g in raw.get("grants") or []], created_on=raw.get("createdOn", ""), updated_on=raw.get("updatedOn", ""), ) diff --git a/sdk/python/tests/test_client.py b/sdk/python/tests/test_client.py index df33d47e..019bb557 100644 --- a/sdk/python/tests/test_client.py +++ b/sdk/python/tests/test_client.py @@ -9,6 +9,7 @@ OrnnClient, OrnnError, SkillDetail, + SkillGrant, SkillSearchResult, UpdateSkillMetadata, ) @@ -606,6 +607,22 @@ def test_set_skillset_permissions_unwraps_envelope(self) -> None: assert result.guid == "ss-1" assert result.is_private is False + @respx.mock + def test_set_skillset_permissions_omits_grants_when_not_passed(self) -> None: + # Pre-#1123 behaviour: no `grants` key on the wire so the server + # falls back to the legacy lists. + import json as _json + + route = respx.put(f"{BASE}/api/v1/skillsets/ss-1/permissions").respond( + 200, + json={"data": {"skillset": _skillset_data(isPrivate=True)}, "error": None}, + ) + with make_client() as ornn: + ornn.set_skillset_permissions("ss-1", is_private=True, shared_with_users=["alice"]) + sent = _json.loads(route.calls.last.request.content) + assert "grants" not in sent + assert sent["sharedWithUsers"] == ["alice"] + @respx.mock def test_delete_skillset_fires_delete(self) -> None: route = respx.delete(f"{BASE}/api/v1/skillsets/ss-1").respond( @@ -687,3 +704,176 @@ def test_search_skillsets_forwards_kind_and_tags(self) -> None: assert params["scope"] == "public" assert result.items[0].kind == "consensus-supported" assert result.items[0].member_count == 2 + + +def _skill_data(**overrides: object) -> dict[str, object]: + base: dict[str, object] = { + "id": "sk-1", + "name": "pdf-extract", + "description": "Extract pdf text", + "isPrivate": True, + "createdBy": "owner-1", + "createdOn": "2026-01-01T00:00:00Z", + "latestVersion": "1.0", + } + base.update(overrides) + return base + + +class TestPermissionGrants: + """Typed ACL grants on skills + skillsets (#1123).""" + + @respx.mock + def test_set_skill_permissions_sends_grants_and_unwraps_skill(self) -> None: + import json as _json + + route = respx.put(f"{BASE}/api/v1/skills/sk-1/permissions").respond( + 200, + json={ + "data": { + "skill": _skill_data( + isPrivate=True, + grants=[{"type": "user", "id": "bob", "level": "read_write"}], + ) + }, + "error": None, + }, + ) + with make_client() as ornn: + result = ornn.set_skill_permissions( + "sk-1", + is_private=True, + grants=[SkillGrant(type="user", id="bob", level="read_write")], + ) + # The typed grant rides on the wire in canonical shape. + sent = _json.loads(route.calls.last.request.content) + assert sent["isPrivate"] is True + assert sent["grants"] == [{"type": "user", "id": "bob", "level": "read_write"}] + # Response `{ skill }` envelope is unwrapped + grants parse back. + assert isinstance(result, SkillDetail) + assert result.grants == [SkillGrant(type="user", id="bob", level="read_write")] + assert route.called + + @respx.mock + def test_set_skill_permissions_omits_grants_when_not_passed(self) -> None: + import json as _json + + route = respx.put(f"{BASE}/api/v1/skills/sk-1/permissions").respond( + 200, json={"data": {"skill": _skill_data()}, "error": None} + ) + with make_client() as ornn: + ornn.set_skill_permissions("sk-1", is_private=False, shared_with_orgs=["acme"]) + sent = _json.loads(route.calls.last.request.content) + assert "grants" not in sent + assert sent["sharedWithOrgs"] == ["acme"] + + @respx.mock + def test_set_skillset_permissions_sends_grants(self) -> None: + import json as _json + + route = respx.put(f"{BASE}/api/v1/skillsets/ss-1/permissions").respond( + 200, + json={ + "data": { + "skillset": _skillset_data( + grants=[{"type": "org", "id": "acme", "level": "read"}] + ) + }, + "error": None, + }, + ) + with make_client() as ornn: + result = ornn.set_skillset_permissions( + "ss-1", + is_private=True, + grants=[SkillGrant(type="org", id="acme", level="read")], + ) + sent = _json.loads(route.calls.last.request.content) + assert sent["grants"] == [{"type": "org", "id": "acme", "level": "read"}] + assert result.grants == [SkillGrant(type="org", id="acme", level="read")] + + @respx.mock + def test_skill_detail_parses_grants(self) -> None: + respx.get(f"{BASE}/api/v1/skills/sk-1").respond( + 200, + json={ + "data": _skill_data( + grants=[ + {"type": "user", "id": "u1", "level": "read"}, + {"type": "org", "id": "o1", "level": "read_write"}, + ] + ), + "error": None, + }, + ) + with make_client() as ornn: + detail = ornn.get("sk-1") + assert detail.grants == [ + SkillGrant(type="user", id="u1", level="read"), + SkillGrant(type="org", id="o1", level="read_write"), + ] + + @respx.mock + def test_skill_detail_defaults_grants_empty_when_absent(self) -> None: + # Pre-#1123 API response carries no `grants` key. + respx.get(f"{BASE}/api/v1/skills/sk-1").respond( + 200, json={"data": _skill_data(), "error": None} + ) + with make_client() as ornn: + detail = ornn.get("sk-1") + assert detail.grants == [] + + +class TestTransferOwnership: + """Ownership transfer for skills + skillsets (#1123).""" + + @respx.mock + def test_transfer_skill_ownership_posts_new_owner_and_unwraps(self) -> None: + import json as _json + + route = respx.post(f"{BASE}/api/v1/skills/sk-1/transfer-ownership").respond( + 200, + json={"data": {"skill": _skill_data(createdBy="alice")}, "error": None}, + ) + with make_client() as ornn: + result = ornn.transfer_skill_ownership("sk-1", "alice") + assert route.calls.last.request.method == "POST" + sent = _json.loads(route.calls.last.request.content) + assert sent == {"newOwnerUserId": "alice"} + assert isinstance(result, SkillDetail) + assert result.created_by == "alice" + + @respx.mock + def test_transfer_skillset_ownership_posts_new_owner_and_unwraps(self) -> None: + import json as _json + + route = respx.post(f"{BASE}/api/v1/skillsets/ss-1/transfer-ownership").respond( + 200, + json={"data": {"skillset": _skillset_data(createdBy="alice")}, "error": None}, + ) + with make_client() as ornn: + result = ornn.transfer_skillset_ownership("ss-1", "alice") + assert route.calls.last.request.method == "POST" + sent = _json.loads(route.calls.last.request.content) + assert sent == {"newOwnerUserId": "alice"} + assert result.created_by == "alice" + + @respx.mock + def test_transfer_skill_ownership_raises_on_invalid_target(self) -> None: + # Target who never signed in to Ornn → 400 invalid_transfer_target. + respx.post(f"{BASE}/api/v1/skills/sk-1/transfer-ownership").respond( + 400, + json={ + "type": "https://github.com/.../ERRORS.md#invalid_transfer_target", + "title": "Invalid transfer target", + "status": 400, + "code": "invalid_transfer_target", + "detail": "unknown Ornn user", + "instance": "/v1/skills/sk-1/transfer-ownership", + }, + ) + with make_client() as ornn: + with pytest.raises(OrnnError) as excinfo: + ornn.transfer_skill_ownership("sk-1", "ghost") + assert excinfo.value.status == 400 + assert excinfo.value.code == "invalid_transfer_target" From 70588d266b0d4451d5208e8eaecf6b3d826298a7 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 02:09:36 +0800 Subject: [PATCH 16/17] docs: permission tiers, grants, and ownership transfer (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - CONVENTIONS §5.2: reconcile the documented↔code drift — replace the fictional `ornn:skill:read/write/admin` catalog with the real route scopes (`ornn:skill:{create,read,update,delete}`, `ornn:skill:build`, `ornn:playground:use`, `ornn:admin:skill`), and frame them as route scopes distinct from object tiers. New §5.4 documents the READ / READ_WRITE / ADMIN object-permission matrix, the typed `grants` ACL shape, and both transfer-ownership endpoints (with the invalid_transfer_target / ownership_conflict behaviors; rides on ornn:skill:update, no new scope). §2.6 + §12 updated. - ERRORS: add `invalid_transfer_target` (400), `ownership_conflict` (409), `invalid_permission_level` (400) with client-action guidance + migration map rows. - ARCHITECTURE: add the `skill.ownership_transferred` analytics event and the `readWriteGrants` count on `skill.permissions_changed`. Part of #1123 --- docs/ARCHITECTURE.md | 3 +- docs/CONVENTIONS.md | 106 ++++++++++++++++++++++++++++++++++++++----- docs/ERRORS.md | 25 +++++++++- 3 files changed, 120 insertions(+), 14 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 556c9768..4cd7623f 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -43,7 +43,8 @@ so dashboards can disambiguate from frontend events of the same name): | `api.skill.published` | skill create + version publish | `skillId`, `skillVersion`, `isNewSkill` | | `user.login` / `user.logout` | session open / close | — | | `skill.created` / `.updated` / `.deleted` / `.version_deleted` | mutation routes | `skillId`, `skillName`, `version`, `adminAction?` | -| `skill.visibility_changed` / `.permissions_changed` | visibility + sharing flips | `skillId`, `isPrivate`, `sharedWithUsers`, `sharedWithOrgs` | +| `skill.visibility_changed` / `.permissions_changed` | visibility + sharing flips | `skillId`, `isPrivate`, `sharedWithUsers`, `sharedWithOrgs`, `readWriteGrants` (count of `read_write` grants, #1123) | +| `skill.ownership_transferred` | ownership handed to another user (#1123) | `skillId`, `skillName`, `priorOwnerId`, `newOwnerId` | | `skill.refresh` / `.source_linked` / `.source_unlinked` | source-pointer ops | `skillId`, `repo`, `ref`, `commit` | | `skill.nyxid_service_tied` / `.agentseal_rescanned` | tie + admin-rescan | `skillId`, `isSystemSkill`, `score` | | `settings.exported` / `.imported` | settings IO | `schemaVersion`, `aggregateStatus`, `dryRun`, `sections` | diff --git a/docs/CONVENTIONS.md b/docs/CONVENTIONS.md index 3e708563..76174eb8 100644 --- a/docs/CONVENTIONS.md +++ b/docs/CONVENTIONS.md @@ -200,6 +200,7 @@ GET /v1/skillsets/{idOrName}/versions — list versions (optional auth) GET /v1/skillsets/{idOrName}/closure — one-call resolve (optional auth) PUT /v1/skillsets/{id} — publish a new immutable version (ornn:skill:update) PUT /v1/skillsets/{id}/permissions — visibility / sharing (ornn:skill:update) +POST /v1/skillsets/{id}/transfer-ownership — hand to another user (ornn:skill:update; ADMIN tier — §5.4) DELETE /v1/skillsets/{id} — delete + cascade versions (ornn:skill:delete) GET /v1/skillset-search — discovery by kind / tags / scope (optional auth) ``` @@ -340,19 +341,25 @@ Endpoint-specific. Rules: - `X-NyxID-Identity-Token` and `X-NyxID-*` headers between proxy and `ornn-api` (internal). - OpenAPI declares one `bearerAuth` scheme; `X-NyxID-*` is not part of the public contract. -### 5.2 Permission strings +### 5.2 NyxID request scopes (route-level) -Format: `ornn::`. - -Actions: `read`, `write`, `admin`, plus resource-specific high-cost actions when needed. +Format: `ornn::`. These are the **request scopes** NyxID +mints onto an access token — `requirePermission(...)` middleware checks them +per route. They gate *who may call an endpoint at all*; they are **distinct +from** the per-object READ / READ_WRITE / ADMIN tier (§5.4), which decides +*what the caller may do to a specific skill/skillset*. A caller needs both: +the route scope to reach the handler, and the object tier to act on the +target. | Permission | Grants | |---|---| | `ornn:skill:read` | Read skills (respects visibility) | -| `ornn:skill:write` | Create, update, delete own skills | -| `ornn:skill:admin` | Manage any skill (override ownership); delete any skill | -| `ornn:skill:generate` | Invoke skill generation endpoints (high LLM cost) | -| `ornn:skill:execute` | Invoke playground chat (runs user code) | +| `ornn:skill:create` | Create skills (upload, pull from GitHub) | +| `ornn:skill:update` | Update / publish / refresh / change permissions / transfer ownership / toggle deprecation / bind NyxID service (+ object ADMIN/READ_WRITE per §5.4) | +| `ornn:skill:delete` | Delete a skill or a single version (+ object ADMIN per §5.4) | +| `ornn:skill:build` | Invoke skill generation endpoints (high LLM cost) | +| `ornn:playground:use` | Invoke playground chat (runs user code) | +| `ornn:admin:skill` | Platform-admin bypass — manage any skill/skillset (override ownership), plus all `/admin/*`, force-audit, and platform settings | | `ornn:category:read` | List categories | | `ornn:category:admin` | Manage categories | | `ornn:tag:read` | List tags | @@ -361,14 +368,90 @@ Actions: `read`, `write`, `admin`, plus resource-specific high-cost actions when | `ornn:activity:read` | Platform activity log read access | | `ornn:stats:read` | Platform-wide dashboard aggregates | -NyxID composes a **"Platform Admin"** role that grants all `*:admin` + `*:read` permissions above; current platform admins inherit this role with zero UX change. Sub-admin roles (content moderator, tag curator, support) can be composed from subsets when needed. +Skillset endpoints **reuse** the `ornn:skill:{create,read,update,delete}` +scopes verbatim (§2.6) — there is no `ornn:skillset:*` scope split in v1. + +NyxID composes a **"Platform Admin"** role around `ornn:admin:skill` (plus the +`*:admin` + `*:read` scopes above); a token carrying `ornn:admin:skill` +bypasses object ownership on every skill/skillset operation. Current platform +admins inherit this role with zero UX change. Sub-admin roles (content +moderator, tag curator, support) can be composed from subsets when needed. -Adding a new permission requires convention-doc update. NyxID role → permission mapping is owned by NyxID config; this doc is the permission catalog. +Adding a new permission requires convention-doc update. NyxID role → +permission mapping is owned by NyxID config; this doc is the permission +catalog. ### 5.3 Scope declaration Every route in OpenAPI tagged with its required scopes. Public endpoints explicitly declare `security: []`. +### 5.4 Object-level permission tiers (#1123) + +Independent of the request scopes above, every **skill AND skillset** carries +a three-tier object-permission model. All three gates derive from one source +of truth — `ornn-api/src/domains/skills/crud/authorize.ts` (`canReadSkill` / +`canWriteSkill` / `canManageSkill`) — and skillsets reuse the same gates. A +request scope decides whether the caller may *call* the endpoint; the object +tier decides whether they may act on *this specific* skill/skillset. + +| Tier | Gate | Who qualifies | What it grants | +|---|---|---|---| +| **READ** | `canReadSkill` | Public skill → anyone. Private → author, platform admin, or any grantee (`read` **or** `read_write`), directly or via a granted org. | View / pull / execute / list versions. | +| **READ_WRITE** | `canWriteSkill` | Author **OR** platform admin **OR** a `read_write` grantee (direct or via a granted org). | READ, plus update the skill's **content + metadata only** (publish a new version). | +| **ADMIN** | `canManageSkill` | Author **OR** platform admin **only**. | Change permissions, transfer ownership, delete skill/version, toggle deprecation, manage dist-tags, bind a NyxID service. | + +A `read_write` grantee is **never** an admin — the danger-zone operations stay +with the author (`createdBy`) and platform admins (`ornn:admin:skill`). Org +grants resolve uniformly: every admin/member of a granted org inherits the +grant's level. The org-membership gates fail soft on an unresolved NyxID +lookup (deny) — they never grant on a "couldn't ask" result. + +#### Typed grant ACL (`grants`) + +The canonical access-control list is a typed `grants` array, exposed on +skill/skillset detail responses and accepted by the permissions endpoints +(`PUT /v1/skills/:id/permissions`, `PUT /v1/skillsets/:id/permissions`): + +```json +{ + "grants": [ + { "type": "user", "id": "", "level": "read" }, + { "type": "org", "id": "", "level": "read_write" } + ] +} +``` + +- `type` — `"user"` (a NyxID person user_id) or `"org"` (a NyxID org user_id). +- `id` — the principal's NyxID id (1..128 chars). +- `level` — `"read"` or `"read_write"`. An invalid value is rejected with + `invalid_permission_level` (a `validation_error` subcode — see `docs/ERRORS.md`). + +The author (`createdBy`) is never represented in `grants` — they hold implicit +ADMIN. The legacy read-only `sharedWithUsers` / `sharedWithOrgs` arrays are +still **accepted** on the permissions endpoints and still **returned** for +back-compat; any principal supplied through them is treated as a `read`-level +grant. A skill predating typed grants authorizes exactly as before. + +#### Ownership transfer + +``` +POST /v1/skills/:id/transfer-ownership { "newOwnerUserId": "" } +POST /v1/skillsets/:id/transfer-ownership { "newOwnerUserId": "" } +``` + +- **Auth:** ADMIN tier (`canManageSkill`) — author or platform admin only. A + `read_write` grantee can never transfer. Rides on the existing + `ornn:skill:update` request scope; **no new scope** was added. +- **Behavior:** immediate, synchronous transfer. The target becomes the new + owner (`createdBy`); the prior owner is kept as a **READ** grantee (retains + visibility, loses edit/admin rights). +- **Target validation:** `newOwnerUserId` must resolve to a known Ornn user — + someone who has signed in to Ornn at least once. An unresolvable target is + rejected with `invalid_transfer_target` (400). +- **No-op guard:** transferring to the current owner returns + `ownership_conflict` (409). +- Returns the updated resource (`{ data: { skill | skillset }, error: null }`). + --- ## 6. SSE streaming @@ -543,7 +626,8 @@ Per-test teardown is the test's responsibility; shared fixtures live in `tests/f - [ ] Error response uses `application/problem+json` with a code from the catalog - [ ] `X-Request-ID` on every response; `requestId` in every error body - [ ] Query params camelCase; arrays as repeated keys; `q` for search -- [ ] Required permissions from the catalog declared in OpenAPI `security` +- [ ] Required request scopes from the catalog declared in OpenAPI `security` (§5.2) +- [ ] Object-level authz, where the target is an owned resource, gates on the READ / READ_WRITE / ADMIN tier (§5.4) — distinct from the route scope - [ ] Content negotiation for multi-representation resources - [ ] SSE events named `_` snake_case - [ ] Deprecation uses RFC 8594 headers diff --git a/docs/ERRORS.md b/docs/ERRORS.md index 92271726..06193901 100644 --- a/docs/ERRORS.md +++ b/docs/ERRORS.md @@ -36,7 +36,7 @@ The pre-#585 `SCREAMING_SNAKE_CASE` shape (`SKILL_NOT_FOUND`, `INVALID_BODY`, ## validation_error **HTTP:** `400 Bad Request` -**Common subcodes (lowercase post-#585):** `invalid_body`, `invalid_query`, `invalid_params`, `invalid_*` (per-field), `empty_body`, `missing_*`, `frontmatter_validation_failed`, `invalid_permissions`, `invalid_zip`, … +**Common subcodes (lowercase post-#585):** `invalid_body`, `invalid_query`, `invalid_params`, `invalid_*` (per-field), `empty_body`, `missing_*`, `frontmatter_validation_failed`, `invalid_permissions`, `invalid_permission_level`, `invalid_transfer_target`, `invalid_zip`, … Request body, query string, or path parameter failed validation. Per-field details are in `errors[]`. @@ -65,6 +65,18 @@ The uploaded payload is not a parseable ZIP — a malformed or unreadable archiv **Client action:** re-create the ZIP and re-upload; do not retry the same bytes. +### invalid_permission_level + +A typed `grants` entry on a permissions request (#1123) carried a `level` outside the allowed set. The only accepted values are `read` and `read_write` (see [`docs/CONVENTIONS.md`](CONVENTIONS.md) §5.4). Surfaced from `PUT /api/v1/skills/{id}/permissions` and `PUT /api/v1/skillsets/{id}/permissions`. + +**Client action:** set every grant's `level` to `read` or `read_write` and retry. + +### invalid_transfer_target + +The `newOwnerUserId` supplied to `POST /api/v1/skills/{id}/transfer-ownership` or `POST /api/v1/skillsets/{id}/transfer-ownership` (#1123) does not resolve to a known Ornn user — the target has never signed in to Ornn, so the directory cannot resolve them. The transfer is rejected before any mutation. + +**Client action:** confirm the target user has signed in to Ornn at least once, then retry with their resolved user id. Do not retry the same id without that change. + --- ## authentication_required @@ -109,7 +121,7 @@ A skill in a dependency closure (#968) could not be resolved — either the refe ## resource_conflict **HTTP:** `409 Conflict` -**Common subcodes (lowercase post-#585):** `skill_name_exists`, `skillset_name_exists`, `skillset_version_exists`, `dependency_cycle`, `dependency_conflict`, `reconcile_already_running`, `redemption_code_expired`, `redemption_code_already_redeemed`, `redemption_code_already_invalidated`, `old_repo_not_confirmed`, … +**Common subcodes (lowercase post-#585):** `skill_name_exists`, `skillset_name_exists`, `skillset_version_exists`, `dependency_cycle`, `dependency_conflict`, `ownership_conflict`, `reconcile_already_running`, `redemption_code_expired`, `redemption_code_already_redeemed`, `redemption_code_already_invalidated`, `old_repo_not_confirmed`, … The request collides with current state — a duplicate skill name on create, a concurrent modification, a job that's already running, etc. @@ -127,6 +139,12 @@ Two different versions of the **same** skill appear in one dependency closure (# **Client action:** align the conflicting pins so every path resolves the skill to the same `` version, then retry. +### ownership_conflict + +A `POST /api/v1/skills/{id}/transfer-ownership` or `POST /api/v1/skillsets/{id}/transfer-ownership` (#1123) named the **current** owner as `newOwnerUserId` — a no-op transfer. The target already owns the resource, so the request is rejected rather than silently succeeding. + +**Client action:** if a transfer is actually intended, supply a different `newOwnerUserId`. If the resource is already owned by the intended user, no action is needed. + --- ## payload_too_large @@ -241,6 +259,9 @@ Clients pinned to the old `SCREAMING_SNAKE_CASE` codes need to switch to the low | _(new in #968)_ | 404 | `skill_dependency_not_found` | `resource_not_found` | | _(new in #968)_ | 409 | `dependency_cycle` | `resource_conflict` | | _(new in #968)_ | 409 | `dependency_conflict` | `resource_conflict` | +| _(new in #1123)_ | 400 | `invalid_permission_level` | `validation_error` | +| _(new in #1123)_ | 400 | `invalid_transfer_target` | `validation_error` | +| _(new in #1123)_ | 409 | `ownership_conflict` | `resource_conflict` | | `UPSTREAM_DOWN` | 502 | `upstream_down` | `upstream_unavailable` | Format rule for future codes: lowercase ASCII, words joined by `_`, no leading/trailing `_`. Pick from the parent §1.4 vocabulary when generic; add a specific subcode only when the caller needs to branch on it. From 90f90fe74b45f2ffda27ff8d199ebc21d1bd17c1 Mon Sep 17 00:00:00 2001 From: Shining <250120269+chronoai-shining@users.noreply.github.com> Date: Tue, 16 Jun 2026 02:27:20 +0800 Subject: [PATCH 17/17] chore(api): regenerate assistant KB digest after CONVENTIONS update (#1123) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The KB digest is built from README/CONVENTIONS/ARCHITECTURE etc.; the §5.2 scope reconciliation + §5.4 permission-tier additions changed the CONVENTIONS source, so the committed digest went stale and the assistant-kb-freshness CI gate failed. Regenerated via `bun run build:assistant-kb`. Part of #1123 --- ornn-api/src/domains/assistant/kb/digest.generated.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ornn-api/src/domains/assistant/kb/digest.generated.md b/ornn-api/src/domains/assistant/kb/digest.generated.md index 57765fca..34dc6225 100644 --- a/ornn-api/src/domains/assistant/kb/digest.generated.md +++ b/ornn-api/src/domains/assistant/kb/digest.generated.md @@ -792,6 +792,7 @@ GET /v1/skillsets/{idOrName}/versions — list versions (optional auth) GET /v1/skillsets/{idOrName}/closure — one-call resolve (optional auth) PUT /v1/skillsets/{id} — publish a new immutable version (ornn:skill:update) PUT /v1/skillsets/{id}/permissions — visibility / sharing (ornn:skill:update) +POST /v1/skillsets/{id}/transfer-ownership — hand to another user (ornn:skill:update; ADMIN tier — §5.4) DELETE /v1/skillsets/{id} — delete + cascade versions (ornn:skill:delete) GET /v1/skillset-search — discovery by kind / tags / scope (optional auth) ``` @@ -811,7 +812,7 @@ POST /v1/skillsets `GET /v1/skillsets/{idOrName}` returns the detail object including the version's `instructions`. -- **Closure:** `GET /v1/skillsets/{idOrName}/closure` resolves `roots = members` through the **same** §2.5 resolver — the union of all members plus each member's transitive dependency closure, deduplicated and topo-sorted (deps-first). The success body carries the version's master prompt as a **root sibling** of `items`: `{ "data": { "instructions": "…", "items": [ … ] }, "error": null }` (the skill `/skills/:id/closure` envelope stays `{ items }`, unchanged). Same error codes as §2.5: `dependency_cycle` (409), `dependency_conflict` (409), `skill_dependency_not_found` (404). Anonymous callers resolving a public skillset whose member transitively pins a private skill get `skill_dependency_not_found` +- **Closure:** `GET /v1/skillsets/{idOrName}/closure` resolves `roots = members` through the **same** §2.5 resolver — the union of all members plus each member's transitive dependency closure, deduplicated and topo-sorted (deps-first). The success body carries the version's master prompt as a **root sibling** of `items`: `{ "data": { "instructions": "…", "items": [ … ] }, "error": null }` (the skill `/skills/:id/closure` envelope stays `{ items }`, unchanged). Same error codes as §2.5: `dependency_cycle` (409), `dependency_conflict` (409), `skill_dependency_not_found` (404). Anonymous callers ---