feat: permission sets for Hypercerts & Certified data#222
Conversation
🦋 Changeset detectedLatest commit: 733f555 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds three namespace-scoped permission-set lexicons, documents how to request and maintain them, and updates code generation so permission-set JSON is excluded from generated outputs while still shipping as raw lexicon files. ChangesPermission set definitions
Documentation and workflow updates
Code generation and export filtering
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 6 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
Adds a new design document describing two AT Protocol permission sets intended to bundle repo-scope permissions for the org.hypercerts.* and app.certified.* record-collection namespaces, so OAuth clients (and CGS) can request a single include:<nsid> scope instead of enumerating per-collection repo: scopes.
Changes:
- Introduces
docs/design/permission-sets.mddocumenting proposed permission-set NSIDs and their expanded scopes. - Enumerates the targeted record collections for each namespace and records design constraints (namespace authority, no wildcards, no
audforrepo:). - Describes publication/resolution flow and CGS consumption patterns.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
5462c64 to
89f2e68
Compare
89f2e68 to
d92f323
Compare
dd5967d to
46843a8
Compare
46843a8 to
6ebc898
Compare
6ebc898 to
0a9d0e9
Compare
0a9d0e9 to
7b80639
Compare
…$(…) Copilot was right that command substitution is the wrong shape here. A `$(…)` arg list silently swallows a producer failure (the script could exit non-zero and lex would still run on partial/empty input and "succeed"). Switch to the pipe + xargs form the original gen-* used: node ./scripts/codegen-lexicon-files.js | xargs -0 -r lex gen-… - `set -o pipefail` makes a producer failure abort the pipeline (verified: a plain pipe reports exit 0 on producer failure; pipefail reports 1). dash — the sh on Ubuntu CI and here — supports it. - `-r` skips lex on empty input; `-0` (NUL-delimited from the script) is robust to any path chars; xargs also sidesteps ARG_MAX. - The script now exits non-zero + writes nothing if it finds no lexicons, so an internal bug surfaces rather than codegenning nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`set -o pipefail` is NOT portable: GitHub's Ubuntu `sh` (dash) rejects it (`set: Illegal option -o pipefail`), which broke CI. And no `xargs` option replaces it — `-r` only skips empty input, it does not propagate a producer's non-zero exit, so a crashed list-generator still yields a "successful" pipeline. Replace the pipe entirely with scripts/run-lex-codegen.js: a single Node process that builds the permission-set-excluded file list and spawns `lex <subcommand> <args> <files…>` with the list as argv, forwarding lex's exit code. One authoritative exit code, no shell builtins, no ARG_MAX (argv, not a command line), no pipefail. Empty list or spawn failure → non-zero exit. (Renamed from codegen-lexicon-files.js, which only printed the list.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…x-codegen) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot is right: spawnSync still calls execve and is subject to the OS argument-length limit. The runner's real wins are avoiding shell pipeline exit-code masking and shell word-splitting, not removing ARG_MAX. Reworded the comments in run-lex-codegen.js and package.json accordingly (the limit is ~2 MB on Linux, far above this few-KB list). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…list The "Adding / modifying a lexicon" steps covered codegen, ERD, README/SKILL, SCHEMAS.md, format, check, and changeset — but never the permission sets. Each authWrite.json enumerates every type:"record" collection in its namespace (the list is both the grant and its boundary; no wildcards allowed inside a set), so a new org.hypercerts.* / app.certified.* record lexicon is NOT grantable via the published set until added to the matching set — and must be removed when deleted. Add a step spelling this out, including the own-namespace-authority rule (each record goes only in its own namespace's set) and the non-record exemption (query/object/permission-set defs are not repo collections). Links to docs/design/permission-sets.md for the full rationale. Prettier also renumbered the list (fixing a pre-existing duplicate-"4" bug). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…nted The repo has a third first-class record namespace — org.hyperboards (board, displayProfile, described in AGENTS.md as the "Hyperboards visual layer") — that had no permission set, so a user's Hyperboards data was not grantable via any published set. Add org.hyperboards.authWrite granting create/update/delete over both org.hyperboards.* record collections, matching the other two namespaces. Verified it enumerates exactly the type:"record" defs on disk and expands to one combined repo: scope. Also update the surrounding docs that the new set and the now-shipped state make stale: - docs/design/permission-sets.md: Status "Proposed" -> "Implemented" (the sets are published lexicon files in this PR, not a proposal); "two sets" -> three throughout, with a new org.hyperboards.authWrite subsection and the three distinct authorities (hypercerts.org, hyperboards.org, certified.app) spelled out; corrected a stray claim that hypercerts/hyperboards "share a domain". - SKILL.md: third table row; "two sets, never one" -> "one set per namespace". - .changeset/permission-sets-crud.md: two -> three sets. - AGENTS.md: the add-a-lexicon permission-set step now lists all three namespaces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… set PR #219 (signature support, HYPER-181) has now merged to main, adding the app.certified.signature.proof record lexicon. This was the documented follow-up: app.certified.authWrite must enumerate every app.certified.* record collection, so the newly-landed collection has to be added or it is not grantable via the published set. Add it (the set now lists all 9 app.certified.* records, verified identical to the type:"record" defs on disk), sync the JSONC example in the design doc, and drop the now-resolved "Known pending update (#219)" note from the Maintenance section. SCHEMAS.md regenerated. npm run check passes (182 tests). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot flagged the bullets under step 9 as unindented, so Markdown renderers treated them as a separate top-level list and the blank line broke the ordered-list numbering. Indent them 3 spaces to match the sub-bullet convention used by the other steps. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…owth semantics Two review points from Ashex on PR #222: - `description` is not a valid field on a `permission-set` def — the spec defines only `title`/`detail` (+ `:lang`) as user-facing consent copy, and it is not shown in the OAuth dialog. It was an AI hallucination in the original set files. Removed from all three sets, their JSONC examples in the design doc, and the generated SCHEMAS.md. All 182 tests still pass, confirming the field was never load-bearing. - Document what happens when a published set later grows: OAuth grants persist the raw `include:<nsid>` and re-expand it on token refresh, so existing sessions gain newly-added (same-authority) collections with no re-auth and no `.vN` NSID; CGS API keys stay a frozen snapshot until re-issued. Grounded in the permission spec and the reference PDS `rotateToken`/`buildTokenScope` path, with a caveat that non-reference Auth Servers should be verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ePDS parity Reframe the 'growing a set' subsection around the real concern: on the OAuth path, adding collections to a published set silently widens every existing grant on next token refresh, with no new consent prompt (verified: rotateToken re-expands and re-issues without a consent re-check). State that plainly, and note the two bounds that make it tolerable — additions are same-namespace by the authority rule (highly correlated with what the user already granted; a set can never silently pull in a cross-namespace permission), and growth only ever adds, never narrows. Guidance: publish under a new set NSID when re-consent is the point. Also record that ePDS inherits this behaviour: it wraps stock @atproto/pds / @atproto/oauth-provider (0.15.10) with a Better Auth login layer and does not fork or override token issuance/refresh, so it persists the raw include:<nsid> and re-expands via the stock rotateToken exactly like the reference PDS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ontext Two Copilot review fixes on the permission-sets PR: - generate-schemas.js: permission-set defs (title/detail/permissions) now render in SCHEMAS.md instead of producing empty header-only sections. - generate-exports.js: extract a readLexicon() helper that rethrows JSON.parse failures with the offending file path, mirroring generate-schemas.js, so codegen errors are traceable. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…owth semantics Two review points from Ashex on PR #222: - `description` is not a valid field on a `permission-set` def — the spec defines only `title`/`detail` (+ `:lang`) as user-facing consent copy, and it is not shown in the OAuth dialog. It was an AI hallucination in the original set files. Removed from all three sets, their JSONC examples in the design doc, and the generated SCHEMAS.md. All 182 tests still pass, confirming the field was never load-bearing. - Document what happens when a published set later grows: OAuth grants persist the raw `include:<nsid>` and re-expand it on token refresh, so existing sessions gain newly-added (same-authority) collections with no re-auth and no `.vN` NSID; CGS API keys stay a frozen snapshot until re-issued. Grounded in the permission spec and the reference PDS `rotateToken`/`buildTokenScope` path, with a caveat that non-reference Auth Servers should be verified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
0e0c2a6 to
83596a9
Compare
Ashex
left a comment
There was a problem hiding this comment.
Approved pending testing per our discussion
83596a9 to
cd08e55
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/generate-schemas.js (1)
275-293: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winOnly handles
repo-type permissions;rpcpermissions would render incomplete.Per the AT Protocol permission spec, a permission's
resourcefield can be"rpc"(withlxm/aud/inheritAudfields) in addition to"repo"(collection/action). This function only renderscollection/action, so anrpcpermission would print just**Resource:** \rpc`with no endpoint/audience detail — not incorrect for the three lexicons in this PR (allrepo-only), but incomplete if a future permission set in this repo addsrpc` permissions.♻️ Optional: extend to cover `rpc` permissions
for (const permission of permissions) { const collections = (permission.collection || []) .map((c) => `\`${c}\``) .join(", "); const actions = (permission.action || []).map((a) => `\`${a}\``).join(", "); output.push(`**Resource:** \`${permission.resource}\``, ""); if (collections) output.push(`**Collections:** ${collections}`, ""); if (actions) output.push(`**Actions:** ${actions}`, ""); + const lxm = (permission.lxm || []).map((l) => `\`${l}\``).join(", "); + if (lxm) output.push(`**LXM:** ${lxm}`, ""); + if (permission.aud) output.push(`**Audience:** \`${permission.aud}\``, ""); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/generate-schemas.js` around lines 275 - 293, generatePermissionSetSection only formats repo-style permissions, so rpc permissions would be incomplete. Update this helper to branch on permission.resource and, for rpc entries, render the rpc-specific fields such as lxm, aud, and inheritAud alongside the resource label; keep the existing collection/action rendering for repo permissions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/design/permission-sets.md`:
- Around line 85-87: The blockquote in the permission-sets documentation is
broken by an empty separator line, which triggers MD028. Update the quoted
section so the gap is either removed or the separator line is also prefixed with
the blockquote marker, keeping the entire paragraph sequence within the same
blockquote near the “Naming” text.
---
Nitpick comments:
In `@scripts/generate-schemas.js`:
- Around line 275-293: generatePermissionSetSection only formats repo-style
permissions, so rpc permissions would be incomplete. Update this helper to
branch on permission.resource and, for rpc entries, render the rpc-specific
fields such as lxm, aud, and inheritAud alongside the resource label; keep the
existing collection/action rendering for repo permissions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3777a009-7424-4d96-8e04-d0a98e383519
📒 Files selected for processing (12)
.agents/skills/building-with-hypercerts-lexicons/SKILL.md.changeset/permission-sets-crud.mdAGENTS.mdSCHEMAS.mddocs/design/permission-sets.mdlexicons/app/certified/authWrite.jsonlexicons/org/hyperboards/authWrite.jsonlexicons/org/hypercerts/authWrite.jsonpackage.jsonscripts/generate-exports.jsscripts/generate-schemas.jsscripts/run-lex-codegen.js
✅ Files skipped from review due to trivial changes (1)
- lexicons/app/certified/authWrite.json
🚧 Files skipped from review as they are similar to previous changes (6)
- lexicons/org/hyperboards/authWrite.json
- lexicons/org/hypercerts/authWrite.json
- package.json
- scripts/generate-exports.js
- SCHEMAS.md
- AGENTS.md
|
Tested in hypercerts-org/ePDS#188 and it looks good:
|
|
(comment generated by Claude Code) Re: CodeRabbit review-summary nitpick — " Stale — no action taken. There is no |
- generate-exports.js: normalize findJsonFiles output to POSIX separators
so downstream split("/") helpers work on Windows (Copilot)
- generate-schemas.js: return consistent {output,...} shape from
generateMainSection's !mainDef early-return branch (Copilot); render
rpc-permission fields (lxm/aud/inheritAud) in generatePermissionSetSection
so rpc permissions aren't reduced to a bare "Resource: rpc" (CodeRabbit)
- permission-sets.md: prefix blockquote separator with > to fix MD028
(CodeRabbit)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
(comment generated by Claude Code) Correction to my earlier comment (above): I wrongly called this nitpick "stale." I searched a stale local checkout that was behind this PR's HEAD and lacked commit Fixed in |
|
This is already published and working - thought we'd already merged this -> merging. |

Summary
Adds permission sets for the
org.hypercerts.*,org.hyperboards.*, andapp.certified.*record collections — both the design and the implementation.A permission set lets any AT Protocol app request a whole bundle of
repo:scopes with oneinclude:<nsid>OAuth scope, instead of enumerating each collection by hand. The user's PDS resolves and expands the set during the OAuth grant; the same published set can also be consumed by services (e.g. the Certified group service) when expanding API-key scopes.One set per namespace, by spec mandate
There are three separate sets — not one combined set — because the spec requires it. A permission set "is limited to expressing permissions that reference resources under the same NSID namespace as the set itself" and "can not address 'sibling groups' or 'parents'".
org.hypercerts,org.hyperboards, andapp.certifiedare different namespace authorities (hypercerts.org,hyperboards.org,certified.app), so a single set legally cannot span them. An app needing more than one requests eachinclude:scope.NSID shape (single segment under the namespace root)
The same authority rule constrains the set's own NSID:
NSID.authorityis the reversed domain of all-but-the-last segment, so the set must be a single leaf directly under the namespace root —org.hypercerts.authWrite, notorg.hypercerts.permissions.crud(whose authoritypermissions.hypercerts.orgwould fail to parentorg.hypercerts.claim.*, makingIncludeScope.toScopessilently return[]). This matches Bluesky's own published sets (app.bsky.authViewAll,app.bsky.authCreatePosts,app.bsky.authDeleteContent,app.bsky.authFullApp— single segment,auth-prefixed leaf) and the spec'scom.example.authFullexamples.Contents
Design —
docs/design/permission-sets.md(newdocs/design/dir): holistic, covering the sets as general AT Protocol artifacts usable by direct OAuth clients, CGS-via-OAuth, and CGS API keys — all consumers of the same published Lexicon source of truth.Implementation — three permission-set lexicons:
lexicons/org/hypercerts/authWrite.json→org.hypercerts.authWriteorg.hypercerts.*record collections (11)lexicons/org/hyperboards/authWrite.json→org.hyperboards.authWriteorg.hyperboards.*record collections (2)lexicons/app/certified/authWrite.json→app.certified.authWriteapp.certified.*record collections (9)Each enumerates every
type: "record"collection in its namespace withaction: [create, update, delete], and expands to a single combinedrepo:scope.Process docs —
AGENTS.md's "Adding / modifying a lexicon" checklist gains a step: when a neworg.hypercerts.*/org.hyperboards.*/app.certified.*record lexicon is added (or removed), update its namespace's permission set to match, since the enumerated collection list is both the grant and its boundary.Design rules followed (spec-cited in the doc)
repo:scopes carry noaud→ requested without?aud=. Noreadaction exists in the scope grammar (records are world-readable), so these are write sets;title/detailare authorization-dialog consent copy.Tooling note
permission-setis a lexicon primitivelex gen-apicannot codegen (it has no TS shape; verified both@atproto/lex-cli@0.9.6and0.10.0throw). These sets need no TS shape — they are the source of truth published to AT Protocol. So they are:lexcodegen invocations viascripts/run-lex-codegen.js— a Node runner that builds the lexicon file list (filtering outdefs.main.type === "permission-set") and spawnslex <subcommand> … <files>with it as argv, forwarding lex's exit code, andscripts/generate-exports.js(same content check),the single content-based exclusion shared by both, so it doesn't break when set files move. They still ship as raw lexicon JSON and appear in
SCHEMAS.md. Fullnpm run checkpasses.(The runner replaced an earlier shell pipeline:
lex … $(node …)swallowed a producer failure, and thenode … | xargsform neededset -o pipefail, which the dash that npm runs scripts with on GitHub's Ubuntu runners rejects. A single Node process gives one authoritative exit code with no shell word-splitting or pipefail dependency.)Rebased onto #219
PR #219 (signature support, HYPER-181) has since merged, adding the
app.certified.signature.proofrecord lexicon. This branch is rebased onto it andapp.certified.signature.proofis now included inapp.certified.authWrite(9 collections), so the set covers everyapp.certified.*record onmain. The earlier "pending follow-up" note has been dropped from the design doc's Maintenance section.Notes
*.authWrite) follow Bluesky'sauth*convention; rename freely.minorchangeset.🤖 Generated with Claude Code
Summary by CodeRabbit
include:<nsid>OAuth scopes.