~ knotless Search/
↑↓ move↵ open⇧↵ all results

chadtmiller.com/pds.js

An atproto PDS in JavaScript, for Cloudflare Workers and Node

0
0

A space scope that names its type alone is granted the type's collections

Merged
Merged chadtmiller.com asked to merge into main from chadtmiller.com/pds.js:fix/expand-space-scopes

What changes

A space: scope that names a space type with no collection, such as space:dev.knotless.space?authority=*&manage=create, is granted the collections the type's lexicon declares. The reference PDS does this as it issues a token (expandSpaceCollections in its LexiconManager).

  • At the request: PAR and the direct authorization path resolve each such type's lexicon through the lexicon resolver, as they resolve include: permission sets. The request keeps the declared collections as spaceTypes. A type that does not resolve to a lexicon of type space, or a server with no resolver, leaves the scope as asked. The reference refuses such a request, but pds.js's own git remote asks for space:dev.pdsjs.git.space bare, and a refusal would end its logins where none ends now.
  • At approval: issueAuthorizationCode writes those collections into the scope (&collection=… for each), after include: expansion. The token, its refresh and the write check all see the concrete grant. requestedScope keeps what the client asked for.
  • New helpers in scope.js: spaceTypeCollections, bareSpaceTypes and expandSpaceScopes.

Why

The write check (spaceScopeMatches) reads grant.collections ?? req.declaredCollections, and no caller supplies declaredCollections. So a bare type's grant permitted no create, update or delete. knotless's login now asks for its space types bare, to keep the loopback client's URL under the browser's Referer limit. Since then, logging in again on pds.js leaves an account unable to write to any of its private repositories, with Scope does not permit create in at://…/space/dev.knotless.space/….

How it was checked

  • test/oauth-endpoints.test.js, new "space scopes that name a type alone":
    • PAR, approval and the token grant …&collection=com.example.post&collection=com.example.reply in the response and in the stored token.
    • An unknown type, and one whose lookup throws, granted as asked beside a known one.
    • The direct path: the collections kept for approval, and every type as asked with no resolver.
  • packages/core/test/space-scope.test.js: the helpers, including a type declaring no collections, a scope that already names some, and space:*.
  • Run here: those files, plus include-scope and space-consent, all pass. packages/git/test/oauth-remote.test.js, which the first push failed, needs better-sqlite3 built for this Node, so CI runs it. tsc (both projects) is clean, and biome is clean on the changed files. The full gate is left to CI.

Shortcomings

A bare space: scope that an include: permission set expands into is not resolved. Only the scopes in the request itself are.

🤖 Generated with Claude Code

Approved

This resolves the lexicon for each bare space:<type> scope at PAR and on the direct authorize path. It refuses unresolvable types with invalid_scope. At approval it writes the declared collections into the scope, after include: expansion, and keeps requestedScope. The helpers are small and handle the edge cases: space:*, scopes that already name collections, and types that declare none. The tests cover the PAR→approve→token path, the refresh token's scope, and both refusal paths. I found nothing that must change. I did not run the tests. I read only the diff, since the repo has no AGENTS.md.

One thing to be aware of: a deployment with no lexicon resolver now refuses every bare space: scope, where before it accepted the scope but granted no write. The pull's tests cover this and it matches the reference PDS, so it looks intended.

Approved

This resolves bare space: types through the lexicon resolver at PAR and on the direct path. It keeps the declared collections as spaceTypes and writes them into the scope in issueAuthorizationCode, after include: expansion. I read the whole diff and found no bug. The helpers handle space:*, scopes that already name collections, and types that declare none. requestedScope is preserved.

The description doesn't match the code. It says a type that doesn't resolve to a space lexicon is refused with invalid_scope, and that a bare type is refused on the direct path when there's no resolver. The code does neither. resolveSpaceTypes swallows resolver errors, drops non-space types, and returns null with no resolver. The tests (grants a type that does not resolve to a space as asked, grants every type as asked with no resolver) and the changeset all assert grant-as-asked. That is also safe, because an unexpanded grant permits no writes. Please either fix the description or make the behaviour match the reference PDS. If refusing is what you want, the code needs to change.

One small thing: a transient resolver failure silently leaves the grant unexpanded. The user would then hit the same "Scope does not permit create" error this pull fixes. A log line would help.

Merged as f0591fd