Skip to content

fix(plugin-sharing): share-link password never leaves the server, is stored with the platform slow hash, and is accepted in a header - #21890

Merged
objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-21839-share-link-password
Oct 6, 2026
Merged

objectstack-fleet[bot] merged 9 commits into
mainfrom
claude/issue-21839-share-link-password

Conversation

@objectstack-fleet

@objectstack-fleet objectstack-fleet Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #21839
Clause-②: no

Server half of the share-link password card. The console transport (sending the password in a header) is the separate objectui card; this PR keeps the query parameter working so the current console is unaffected until that lands.

What changed

All in packages/plugins/plugin-sharing/src, plus one changeset and a regenerated census page.

  1. The stored hash never leaves the server. ShareLinkService.createLink returned the row it had just inserted, hash included, and both mounts of POST /api/v1/share-links (this plugin's route and the runtime dispatcher twin) answer with that return value. It now returns the row through one exit projection, withoutPasswordHash. listLinks and resolveToken pass their results through the same projection. They already came from engine reads that strip the internal column, so the projection holds whichever engine is wired. The audit ledger and engine write responses already omit internal fields, so this PR does not change them.
  2. The stored form is the platform's slow password hash. New module share-link-password.ts: scrypt with the parameters account passwords use (N=16384, r=16, p=1, 64-byte key, 16-byte salt as hex, NFKC). It is built on node:crypto with no new dependency, and new rows are stored as scrypt$SALT$KEY. The two legacy forms (sha256$… and the weak$… no-SubtleCrypto fallback) still verify. resolveToken re-hashes a legacy row into the current form after a successful verification, but only once every later gate (standing policy, record existence, eligibility) has passed. A switched-off or ineligible link therefore takes no write. Every comparison uses timingSafeEqual, and the plaintext legacy form is compared through a digest of both sides. A refused upgrade write does not block the read, because the legacy form still verifies. It is reported once per instance at error, naming the link only. A deployment that injects its own hashPassword/verifyPassword pair is left alone. The old default could also write a weak$ row on a runtime without SubtleCrypto; it no longer can.
  3. The password travels in a header. One helper, presentedPassword, now reads the password for both public routes. /:token/resolve already accepted x-share-password. /:token/messages on this mount read only ?password= and now accepts the header too, as its runtime twin always has. The query parameter is still accepted for compatibility (named in the changeset). Neither form is logged on this path: the routes write no log line, the service's log lines name the link and never the presented password, and the Hono adapter's failure log records the path without the query string.

Tests

New src/share-link-password.test.ts, 19 cases on a real ObjectQL + driver-sql + better-sqlite3, so the engine's strip is live:

  • No exit carries the password_hash key or any piece of the stored hash: mint (service and POST route), list (service and route), redemption (service and route).
  • Stored form matches scrypt$ + 32 hex + $ + 128 hex, holds no plaintext, and is salted per row. The verifier refuses wrong, empty and unknown-form inputs.
  • For both legacy forms (salted SHA-256, plaintext): still verifies; a wrong or missing password is refused and writes nothing; the right password upgrades the row to scrypt$; the upgraded hash verifies the same password and still refuses a wrong one.
  • No upgrade on a switched-off object. No upgrade with an injected hasher pair. A refused upgrade still serves the read, leaves the legacy form in place, is reported exactly once with { link, reason }, and no log carries the password or the hash.
  • The list and the redemption result carry no hash even from an engine that does not strip it (a control asserts that engine does return it), so the service's own projection is pinned.
  • The header is accepted on /resolve and /messages, and so is ?password=. A wrong password is refused for both forms on both routes, with ADR-0112 code + success: false asserted. No log line carries the presented password on any outcome.

Local runs, final head eacae6b6be unless noted (eacae6b6be adds the non-stripping-engine pin to the test file):

  • pnpm --filter @objectstack/plugin-sharing test at head eacae6b6be: 39 files / 973 tests passed.
  • pnpm --filter @objectstack/plugin-sharing typecheck (src + scripts + check:test-typecheck): green; the test layer holds the existing 2 files / 3 pinned signatures, with nothing new.
  • pnpm --filter @objectstack/runtime exec vitest run --maxWorkers=2 src/domains/share-links (the dispatcher twin's suites, against the rebuilt plugin dist/): 2 files / 29 tests passed.
  • Lint narrowed to the 4 changed .ts files: eslint --no-inline-config --format json reports 4 files, 0 errors, 0 warnings. The population is read from eslint.config.mjs: **/*.{ts,…} covers all four. Invariance: the config uses no typed linting (no parserOptions.project), so this diff cannot move any verdict on an untouched file.
  • dispatch-gates --ran: 95 of 97 derived families run, all exit 0. Two are NOT MEASURED, both PREREQUISITE NOT MET (exit 3): check:dual-build-cjs-loads needs a whole-repo build, and check:i18n needs the CLI build closure. This diff changes no object, label or translation source. Both are declared to CI.

Ablations (each mutation made with scripts/ablation-replace.mjs, which confirmed the change on disk; restored to the HEAD blob each time, confirmed by matching hashes and an empty git diff HEAD):

  • the withoutPasswordHash projection removed: 2 failed / 17 passed (the mint pin and the non-stripping-engine pin).
  • legacy upgrade disabled: 3 failed / 15 passed (both legacy-row pins and the refused-upgrade pin).
  • header read removed from presentedPassword: 4 failed / 14 passed (both header pins, the header wrong-password pin, the redemption-route pin).

Acceptance notes

  • Twin precedence. When both forms arrive, the query parameter is read first, on both mounts: the runtime dispatcher twin (packages/runtime/src/domains/share-links.ts, outside this card's file surface) reads it that way, and two mounts of the same routes must not answer differently. The header is the form clients should send. Making the header win on both mounts is a small follow-up for whoever retires the query form. Carrier: the objectui console card, whose landing is the point the query parameter can go.
  • Cost per protected redemption. Verifying a protected link's password now costs one slow-hash evaluation per request, by design and at the same cost as a sign-in.
  • The spec contract type is unchanged. ShareLink.password_hash stays declared (optional) in packages/spec/src/contracts/share-link-service.ts, as the persisted-shape mirror. Only the runtime value leaving the service drops it, so no published type narrows. See Clause-② above.
  • Census. The upgrade write is one more engine write call site, so node scripts/tenant-audit-census.mjs --write regenerated content/docs/permissions/tenant-audit-census.mdx and its counts file. The page's hand-written figures moved from 232 to 233 (decidable 154 to 155, elevated 113 to 114). check:tenant-audit-census and its self-test are green.

Generated by Claude Code

claude added 4 commits October 5, 2026 14:22
…w hash with legacy upgrade

Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-authored-by: Claude <noreply@anthropic.com>
…acy upgrade and transport

Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-authored-by: Claude <noreply@anthropic.com>
…ash-upgrade write site

Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/plugin-hono-server, @objectstack/plugin-sharing, @objectstack/runtime, touching 43 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-sharing/package.json), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

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

  • content/docs/permissions/system-context.mdx (via createLink (symbol, a method of class ShareLinkService), listLinks (symbol, a method of class ShareLinkService))
  • content/docs/protocol/kernel/http-protocol.mdx (via DEFAULT_CORS_ALLOW_HEADERS (symbol, a top-level const object), /:token/resolve (route, a path literal in a comment on a changed line), /share-links/:token/resolve (route, a path literal in a comment on a changed line))
  • content/docs/protocol/kernel/i18n-standard.mdx (via /:token/messages (route, a path literal in a comment on a changed line))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

  • content/docs/releases/v14.mdx (via createLink (symbol, a method of class ShareLinkService), /:token/resolve (route, a path literal in a comment on a changed line))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-sharing/package.json) — pages documenting those are invisible to this run
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 32 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 cab639671528ef6f3a201e8995794378a4a28bfe → packageMentionDocs.

Which tree this was computed on

This run read content/docs from e5c1ab18cf4a5875e784918135c0c4777b536304 — the merge of head c852449b1b6822d07a7e1b39f5f74869cffc99d8 into base cab639671528ef6f3a201e8995794378a4a28bfe, 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 e5c1ab18cf4a5875e784918135c0c4777b536304 && git checkout e5c1ab18cf4a5875e784918135c0c4777b536304
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin cab639671528ef6f3a201e8995794378a4a28bfe c852449b1b6822d07a7e1b39f5f74869cffc99d8 && git checkout -B drift-repro cab639671528ef6f3a201e8995794378a4a28bfe && git merge --no-ff c852449b1b6822d07a7e1b39f5f74869cffc99d8

node scripts/docs-audit/affected-docs.mjs --json cab639671528ef6f3a201e8995794378a4a28bfe

⚠️ 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 cab639671528ef6f3a201e8995794378a4a28bfe → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@github-actions github-actions Bot added documentation Improvements or additions to documentation tests tooling labels Oct 5, 2026
claude added 3 commits October 5, 2026 16:10
…ngine that does not strip the hash

The list and redemption pins passed with the projection removed, because the
engine's internal-column strip already held them. This case wires an engine
whose reads hand the hash back (with a control asserting it does), so the
projection itself is what the assertion reads.

Co-authored-by: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
…ssword header passes CORS, and hashing works in WebContainer

- X-Share-Password joins DEFAULT_CORS_ALLOW_HEADERS so a cross-origin client
  can use the header form.
- Both public share-link routes answer Cache-Control: no-store and
  Vary: X-Share-Password on every outcome, on both mounts.
- On WebContainer the password key is derived by @noble/hashes scrypt with
  the same parameters and stored form as node:crypto; hashes interchange.

Co-Authored-By: Claude <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
claude added 2 commits October 5, 2026 21:03
…public route with no-store

The public-route header wrapper now maps a throw from outside the body's own
try (service resolution) through errorFromThrown before adding the headers;
authenticated routes keep propagating. Adds an NFKC-differing password case to
the cross-implementation scrypt test, both directions, and lists
@objectstack/hono in the changeset frontmatter its text already names.

Claude-Session: https://claude.ai/code/session_018zT8d8NpiQ1ExhuNd5TxY6
Co-authored-by: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

2 participants