feat: skill permission levels (read / read-write) + ownership transfer (#1123)

.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.

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` |

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:<resource>:<action>`.
-
-Actions: `read`, `write`, `admin`, plus resource-specific high-cost actions when needed.
+Format: `ornn:<resource>:<action>`. 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": "<nyxid-person-user-id>", "level": "read" },
+    { "type": "org",  "id": "<nyxid-org-user-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": "<id>" }
+POST /v1/skillsets/:id/transfer-ownership    { "newOwnerUserId": "<id>" }
+```
+
+- **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 `<resource>_<event>` snake_case
 - [ ] Deprecation uses RFC 8594 headers

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 `<major.minor>` 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.

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
@@ -751,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 ----
@@ -770,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 ----

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
 
 ---