Skip to content

fix(plugin-sharing): a plain member's own share-link list is self-scoped (ADR-0111) - #21403

Merged
objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-21328-share-links-self-list
Oct 2, 2026
Merged

objectstack-fleet[bot] merged 5 commits into
mainfrom
claude/issue-21328-share-links-self-list

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #21328
Clause-②: no

What was wrong

GET /api/v1/share-links refused every plain member: a caller holding member_default and no sys_share_link grant was refused with no filter, with an object filter, and with any record. The console's Share dialog loads this list when it opens, so it failed for that whole class of user on every record. Both share-link doors already force createdBy to the caller, because ADR-0111's surface table rules the list self-scoped. But ShareLinkService.listLinks read sys_share_link under the caller's context, and that read needs an object-level grant the member baseline does not carry. An admin's list answered.

Measured through this PR's route pin (showcase boot via @objectstack/verify). At the merge base ee75aae1a, and again with the elevation ablated at 8682b7b24, the plain member's six list cases answered 500 with code PERMISSION_DENIED. The admin's list answered 200 with exactly the admin's own link. The Acceptance notes explain why this composition answers 500 where the card's hosted composition answered 403.

The change (the triage direction quoted on the card)

  • One creator rule. isLinkCreator(row, context) in share-link-service.ts holds when the caller has a non-empty user identity and the row's created_by equals it. revokeLink and listLinks both read this rule, and there is no second creator test.
  • listLinks. The read runs under the system context only when the creator filter passes that rule, which means the filter names the caller. In that case the created_by constraint is the caller's identity, written server-side, and each returned row must pass isLinkCreator before it leaves. Every other shape keeps the caller's context: no creator filter, another user as creator, an empty or absent identity, and an admin listing someone else's links. A system caller keeps its bypass. listLinks keeps its single isSystem read, so the system-context census stays at 114.
  • Projection unchanged. The engine strips both internal columns under any context. The token comes back through the existing privileged accessor, and the password hash never does (route pin [secret]).
  • ⛔ No permission-set edit. The member still holds no sys_share_link grant (route pins [persona] and [no-grant]).

What this deliberately changes beyond the list

  • revokeLink's creator check now refuses a caller with no user identity. Before, undefined === undefined let an identity-less, non-system caller revoke a link whose created_by the driver omitted. Neither HTTP door reaches that case, because both answer 401 first. The new behaviour is pinned fail-closed. The creator, non-creator and ADR-0111 D8 record-manager paths are unchanged, and their existing pins pass untouched.
  • share-link-enforcement-context.test.ts. The pin listLinks reads under the same whole envelope read the envelope off the sys_share_link read. The route's list is always the caller's own, and that list is now the system read, so the pin now observes the envelope at the listLinks call. It checks that every key the resolver produced arrives, using the same comparison as the createLink pin, and asserts that the read is the self-scoped one. The property the file pins is unchanged: the route hands enforcement the whole envelope (ruling A, commit 8e13ca8).
  • @objectstack/spec. The IShareLinkService.listLinks doc comment said every listing is read under context. It now describes the self-scoped own list. This is a doc comment only.

Tests (head 8682b7b24)

  • Route pin, real composition. packages/qa/dogfood/test/share-links-self-list.dogfood.test.ts has 11 cases, and all 11 pass. A plain member lists exactly their own links, both with no filter and with the Share dialog's object and record filter. Member B's links and the admin's sit on the same record and stay absent. A createdBy query naming B is ignored. A plain member with no links gets []. The row carries its token and no password_hash. The admin's list is unchanged, and an anonymous caller gets 401 UNAUTHENTICATED.

  • Service pins. share-link-service.test.ts and share-link-enforcement-context.test.ts pass 52 of 52. The service pins cover:

    • a member's own list;
    • the record filter;
    • a foreign creator and an unfiltered list, both refused under the caller's context with PERMISSION_DENIED / 403;
    • no identity (absent, and empty), which never takes the elevated path;
    • the admin path;
    • the system bypass;
    • a dropped created_by predicate, which still leaks no foreign row;
    • revoke and list reading one rule.
  • Package runs. pnpm --filter @objectstack/plugin-sharing test passes 928 tests in 38 files, and its typecheck is OK. @objectstack/spec passes typecheck, and its test passes 17529 tests in 598 files. @objectstack/dogfood passes typecheck, and its tsc program includes the new test.

  • Ablations, service level. Each leg ran from the committed head through scripts/ablation-replace.mjs, and each restore was proven to leave the blob equal to HEAD with git diff HEAD empty.

    Mutation Red Green
    Identity check removed from isLinkCreator 3: no-identity list, no-identity revoke, system bypass 49
    Server-side constraint removed (identity where and creator post-filter) 6, including other creators' links on the same record 46
    Identity where only removed 1 51
    Creator post-filter only removed 1 (the dropped-predicate pin) 51
    Elevation removed 5: the own-list pins and the envelope-call pin 47
  • Ablation, route level. The elevation was removed and plugin-sharing rebuilt. ablation-dist-preflight found the marker in 2 dist files. Result: 6 red (the own, empty, secret and foreign cases, at 500 PERMISSION_DENIED) and 5 green (persona, no-grant, fixture, admin, anonymous, which the test header says sit still). For the restore leg, plugin-sharing was rebuilt, the marker was absent from dist, and all 11 passed. The first route mutation (a false && guard) was constant-folded out of dist. The preflight caught that before any test ran, so it measured nothing, and the leg was redone with a marker the bundler keeps.

  • Gates. dispatch-gates --commands at this head derives 89 commands, and all exit 0. check:dual-build-cjs-loads first exited 3 (PREREQUISITE NOT MET, because unrelated packages had not been built). Those packages were built from the turbo cache, and the rerun exited 0. The --ran reconciliation reports 89 derived, 89 run and 0 NOT MEASURED, a figure derived from the recorded exit codes.

  • Lint (narrowed). eslint --no-inline-config over the 5 changed TS files reports 0 errors and 0 warnings in all 5 files, and the config resolves for each. The narrowing is safe because the config enables no type-aware rules, and its local plugins read only their baselines. So this diff cannot move the verdict on any untouched file.

Docs

content/docs/** (outside releases/ and references/) and skills/** hold no sentence this change makes false. It makes true ADR-0111's surface-table entry for share links, "list is self-scoped", for every signed-in caller.

Acceptance notes

  • Which door served. In this composition, plugin-sharing's registerShareLinkRoutes serves /api/v1/share-links, not the runtime /share-links domain. The error status shows it: that door answers 500 where the dispatcher's mapper answers 403. The runtime domain forwards createdBy as the caller's id and the full envelope to the same listLinks, so the service fix covers both doors. The runtime door has no separate route pin here.
  • Finding, for the seat to file (a defect of a different class, not fixed here). The plugin door's catch maps the security middleware's PERMISSION_DENIED refusal (which carries statusCode 403 and no status) to HTTP 500. It is still reachable after this PR: POST /api/v1/share-links by a plain member, on an object they cannot read, answered 500 with code PERMISSION_DENIED on the showcase boot.
  • The own list is not organization-walled. The self-scoped read runs under the system context, so it sits outside the tenant wall, as revokeLink's creator read already does. Share-link rows are inserted under the system context with organization_id null (measured on the showcase boot). A caller's own list therefore spans organizations in the environment's database, the same reach revokeLink already gives the creator. This follows the ruled direction. It is an observation only, with no wrong answer measured.
  • No ADR anchor added. There is no scripts/adr-anchors entry for share-link-service.ts. The ADR-0111 id stays in the code. An anchor would be outside the claimed surface, and no one is set to carry it.
  • Out of scope. Who may mint a link ([Decision] share-links mint authority: the owner of a record on an access: private object can never mint a share link — admit the share-manager (canManageShares) beside the visibility read? (amends ADR-0111 D8 rule 1) #21329) is not addressed here.
  • Base not merged. origin/main has moved since the merge base. None of the upstream changes touch this PR's six files.

Generated by Claude Code

claude added 5 commits October 2, 2026 10:13
Route-level pin through the real runtime composition: a plain member
lists exactly their own links, foreign creators stay absent, the admin
path and the anonymous 401 are unchanged.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
listLinks read sys_share_link under the caller's context, so the
self-scoped list ADR-0111 rules (both doors force createdBy to the
caller) demanded an object-level grant the member baseline does not
carry, and every plain member's list was refused.

The caller's own list (a non-empty user identity, and the creator
filter equal to it) is now read under the system context, constrained
server-side to that identity, and every row must pass the creator rule
before it leaves. One helper, isLinkCreator, is that rule for both
listLinks and revokeLink. Every other shape keeps the caller's context.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
…coped list on the contract

The elevation predicate no longer reads isSystem: a system caller asking
for its own links gets the same rows either way, and listLinks keeps the
single isSystem read site it had. The IShareLinkService.listLinks doc
comment said every listing is read under context; it now states the
self-scoped own list. Adds the persona pin to the route test and the
changeset.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
The route's list is always the caller's own, which is now the
self-scoped system read, so the envelope pin reads what the route hands
listLinks (every resolver key) instead of the context of a read that no
longer carries it.

Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Oct 2, 2026
@github-actions

github-actions Bot commented Oct 2, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-sharing, @objectstack/spec, touching 4 documentable anchor(s).

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/system-context.mdx (via listLinks (symbol, a method of class ShareLinkService), revokeLink (symbol, a method of class ShareLinkService))
What this run could not see
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 138 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 85986144c2ef6f379955137677c5cbfb00e194d2 → packageMentionDocs.

Which tree this was computed on

This run read content/docs from 37ca7de54a5c4369209615168125af7fe83f9184 — the merge of head 8682b7b24115599b49aaba02fd05777ecc0c5a0d into base 85986144c2ef6f379955137677c5cbfb00e194d2, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 37ca7de54a5c4369209615168125af7fe83f9184 && git checkout 37ca7de54a5c4369209615168125af7fe83f9184
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 85986144c2ef6f379955137677c5cbfb00e194d2 8682b7b24115599b49aaba02fd05777ecc0c5a0d && git checkout -B drift-repro 85986144c2ef6f379955137677c5cbfb00e194d2 && git merge --no-ff 8682b7b24115599b49aaba02fd05777ecc0c5a0d

node scripts/docs-audit/affected-docs.mjs --json 85986144c2ef6f379955137677c5cbfb00e194d2

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 85986144c2ef6f379955137677c5cbfb00e194d2 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 2, 2026 12:31
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 2, 2026 12:31
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 2, 2026
Merged via the queue into main with commit db3fee3 Oct 2, 2026
44 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-21328-share-links-self-list branch October 2, 2026 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/l tests tooling

Projects

None yet

2 participants