Add atproto-oauth skill#6
Conversation
📝 WalkthroughWalkthroughAdds a new AT Protocol OAuth 2.1 skill document ( ChangesAT Protocol OAuth Skill Addition
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
|
|
||
| ## Verifying against current docs | ||
|
|
||
| Endpoint signatures, scope grammar, and token formats can change. If an MCP client with AT Protocol documentation access is available, use it to check current details rather than relying solely on this skill. The AT Protocol docs MCP server is available at `https://atproto.mcp.kapa.ai` if one isn't already connected. |
There was a problem hiding this comment.
This can be dropped if it's too large a shift from the skills approach, in my harness I include a trailing section to always verify knowledge with an mcp.
|
just a quick thought i think it might overlap quite a bit with the epds login skill as well: https://github.com/hypercerts-org/ePDS/tree/main/.agents/skills/epds-login. But i guess this is also more generic to normal pds oauth so i guess it fine? Do let me know what you think once you look into epds-login and if you think it makes sense to have this here as well then we can merge it |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 @.agents/skills/atproto-oauth/SKILL.md:
- Around line 56-68: The OAuth skill examples still use transition:generic as if
it were the default scope, which conflicts with the later migration-only
guidance. Update the normative example in SKILL.md and the later inline example
to use the preferred modern scope instead, and keep transition:generic only in a
clearly labeled legacy migration example. Refer to the example JSON snippet and
the later inline example so both occurrences stay consistent.
- Around line 24-46: The fenced blocks in SKILL.md are missing a language tag,
triggering markdownlint MD040 and reducing readability. Update the three
affected fences in the atproto-oauth skill content to use a plain text language
tag, keeping the existing prose and structure unchanged. Locate the code fences
in the numbered workflow section and apply the same fix consistently to all
three instances.
In `@README.md`:
- Around line 31-37: The README’s hosted-skills section is currently empty, so
the newly hosted skill is not discoverable. Update the “Included skills (hosted
here)” section to add a bullet for the hosted skill using its actual name, such
as atproto-oauth, or remove the subsection entirely until it contains real
entries. Keep the change localized to the README section that lists hosted
skills so the skill is visible to readers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1c86a738-9f5c-47ca-82e6-9e823af5caea
📒 Files selected for processing (4)
.agents/skills/atproto-oauth/SKILL.mdCONTRIBUTING.mdREADME.mdskills/hypercerts/references/skill-map.md
| ``` | ||
| 1. Resolve identity handle/DID → DID document → PDS URL (via #atproto_pds service) | ||
| 2. Discover PDS GET {PDS}/.well-known/oauth-protected-resource | ||
| → authorization_servers[0] (must be exactly one) | ||
| 3. Discover AS GET {AS}/.well-known/oauth-authorization-server | ||
| → issuer, authorization_endpoint, token_endpoint, | ||
| pushed_authorization_request_endpoint, | ||
| require_pushed_authorization_requests=true, | ||
| authorization_response_iss_parameter_supported=true, | ||
| client_id_metadata_document_supported=true, | ||
| dpop_signing_alg_values_supported ⊇ [ES256], | ||
| code_challenge_methods_supported ⊇ [S256] | ||
| 4. PAR POST {par_endpoint} with client assertion (confidential) + DPoP proof | ||
| → 400 use_dpop_nonce on first try → retry with nonce → 201 request_uri | ||
| 5. Redirect GET {authorize_endpoint}?client_id=...&request_uri=... (nothing else) | ||
| 6. User approves on AS | ||
| 7. Callback GET {redirect_uri}?code=...&state=...&iss=... | ||
| verify state exists (delete row = single-use), verify iss == stored issuer | ||
| 8. Token exchange POST {token_endpoint}: code + code_verifier + client assertion + DPoP | ||
| → access_token, refresh_token, scope, sub (DID), token_type=DPoP | ||
| 9. Identity verification sub → DID doc → #atproto_pds → matches PDS from step 2 → AS matches step 3 | ||
| 10. Resource requests Authorization: DPoP <access_token> + DPoP proof with ath=SHA-256(access_token) | ||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language tag to the fenced examples.
These three code fences trip markdownlint (MD040) and are harder to scan without a language. text is sufficient for each.
Suggested change
-```
+```textApply the same change to the other two fences.
Also applies to: 83-86, 118-122
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 24-24: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🪛 SkillSpector (2.3.7)
[error] 149: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🤖 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 @.agents/skills/atproto-oauth/SKILL.md around lines 24 - 46, The fenced
blocks in SKILL.md are missing a language tag, triggering markdownlint MD040 and
reducing readability. Update the three affected fences in the atproto-oauth
skill content to use a plain text language tag, keeping the existing prose and
structure unchanged. Locate the code fences in the numbered workflow section and
apply the same fix consistently to all three instances.
Source: Linters/SAST tools
| ```json | ||
| { | ||
| "client_id": "https://example.app/oauth-client-metadata.json", | ||
| "application_type": "web", | ||
| "grant_types": ["authorization_code", "refresh_token"], | ||
| "response_types": ["code"], | ||
| "scope": "atproto transition:generic", | ||
| "redirect_uris": ["https://example.app/oauth/callback"], | ||
| "dpop_bound_access_tokens": true, | ||
| "token_endpoint_auth_method": "private_key_jwt", | ||
| "token_endpoint_auth_signing_alg": "ES256", | ||
| "jwks_uri": "https://example.app/.well-known/jwks.json" | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Don't teach transition:generic as the default path.
The normative examples still include the coarse transition scope, which conflicts with the later guidance to reserve transitional scopes for legacy ports. Keep it only as a clearly labeled migration-only example.
Suggested change
- "scope": "atproto transition:generic",
+ "scope": "atproto repo:app.bsky.feed.post?action=create",Apply the same replacement in the later inline example at Line 96.
Also applies to: 96-97
🧰 Tools
🪛 SkillSpector (2.3.7)
[error] 149: [PE3] Credential Access: Code accesses credential files (SSH keys, AWS credentials, etc.). This could indicate credential theft attempts.
Remediation: Remove references to credential paths. Use environment variables or secrets managers. For docs, use placeholder paths (e.g., /path/to/config). Never load .env or token files in production code paths.
(Privilege Escalation (PE3))
🤖 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 @.agents/skills/atproto-oauth/SKILL.md around lines 56 - 68, The OAuth skill
examples still use transition:generic as if it were the default scope, which
conflicts with the later migration-only guidance. Update the normative example
in SKILL.md and the later inline example to use the preferred modern scope
instead, and keep transition:generic only in a clearly labeled legacy migration
example. Refer to the example JSON snippet and the later inline example so both
occurrences stay consistent.
| ## Included skills (hosted here) | ||
|
|
||
| The meta-skill points to: | ||
| Generic, reusable skills that are not tied to any specific app live directly in this repository under `.agents/skills`. | ||
|
|
||
| ## Included pointers (hosted elsewhere) | ||
|
|
||
| App-specific focused skills remain pointers to their source repositories: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Surface the newly hosted skill.
The new hosted-skills subsection is empty, so atproto-oauth still isn't discoverable from the README. Add a bullet here or remove the subsection until it lists actual items.
Suggested change
## Included skills (hosted here)
Generic, reusable skills that are not tied to any specific app live directly in this repository under `.agents/skills`.
+
+- `atproto-oauth` — AT Protocol OAuth 2.1 guidance📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## Included skills (hosted here) | |
| The meta-skill points to: | |
| Generic, reusable skills that are not tied to any specific app live directly in this repository under `.agents/skills`. | |
| ## Included pointers (hosted elsewhere) | |
| App-specific focused skills remain pointers to their source repositories: | |
| ## Included skills (hosted here) | |
| Generic, reusable skills that are not tied to any specific app live directly in this repository under `.agents/skills`. | |
| - `atproto-oauth` — AT Protocol OAuth 2.1 guidance | |
| ## Included pointers (hosted elsewhere) | |
| App-specific focused skills remain pointers to their source repositories: |
🤖 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 `@README.md` around lines 31 - 37, The README’s hosted-skills section is
currently empty, so the newly hosted skill is not discoverable. Update the
“Included skills (hosted here)” section to add a bullet for the hosted skill
using its actual name, such as atproto-oauth, or remove the subsection entirely
until it contains real entries. Keep the change localized to the README section
that lists hosted skills so the skill is visible to readers.
2 participants
Add a more generic atproto-oauth skill that can be used anywhere. Using this skill will prevent the use of outdated knowledge such as relying on transition scopes but also build in institutional knowledge about XRPC/RPC methods (particularly when using app.bsky RPC).
Summary by CodeRabbit