AtProtoScopes.Repo silently widens RepoAction.None to a full write grant #94

New issue

Closed

opened 2026-08-20 21:14:23 +00:00 by claude-bot · 2 comments

claude-bot commented

2026-08-20 21:14:23 +00:00

Collaborator

AppendRepoActions in src/ATProtoNet/Auth/OAuth/AtProtoScopes.cs:567 returns early for both RepoAction.All and RepoAction.None:

if (actions is RepoAction.All or RepoAction.None)
    return;

Since an omitted action= list means the full default set in the scope grammar, AtProtoScopes.Repo("app.bsky.feed.post", RepoAction.None) returns repo:app.bsky.feed.post — a grant of create + update + delete, the exact opposite of what the caller asked for. It affects both Repo(string, RepoAction) (line 202) and Repo(IReadOnlyList<string>, RepoAction) (line 219).

This is the same defect that was just fixed for AtProtoScopes.Space(...) on PR #90, where SpaceAction.None now throws an ArgumentException because the grammar has no way to express an empty action list. It was left out of that PR because Repo is shipped public API and changing it from "returns a string" to "throws" is a binary-compatible but behaviourally breaking change that needs its own Breaking changes CHANGELOG entry.

Suggested fix: throw ArgumentException(nameof(actions)) for RepoAction.None in both overloads, add a test, and note the change under Breaking changes.

Filed by Claude while working on #90 (run).

`AppendRepoActions` in `src/ATProtoNet/Auth/OAuth/AtProtoScopes.cs:567` returns early for both `RepoAction.All` and `RepoAction.None`: ```csharp if (actions is RepoAction.All or RepoAction.None) return; ``` Since an omitted `action=` list means the full default set in the scope grammar, `AtProtoScopes.Repo("app.bsky.feed.post", RepoAction.None)` returns `repo:app.bsky.feed.post` — a grant of create + update + delete, the exact opposite of what the caller asked for. It affects both `Repo(string, RepoAction)` (line 202) and `Repo(IReadOnlyList<string>, RepoAction)` (line 219). This is the same defect that was just fixed for `AtProtoScopes.Space(...)` on PR #90, where `SpaceAction.None` now throws an `ArgumentException` because the grammar has no way to express an empty action list. It was left out of that PR because `Repo` is shipped public API and changing it from "returns a string" to "throws" is a binary-compatible but behaviourally breaking change that needs its own `Breaking changes` CHANGELOG entry. Suggested fix: throw `ArgumentException(nameof(actions))` for `RepoAction.None` in both overloads, add a test, and note the change under `Breaking changes`. --- _Filed by Claude while working on #90 ([run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/316))._

claude-bot referenced this issue

2026-08-20 21:14:23 +00:00

Claude: changes for issue #89 #90

claude-bot was assigned by Grandiras

2026-08-20 21:16:13 +00:00

claude-bot commented

2026-08-20 21:16:22 +00:00

Author

Collaborator

🤖 On it — follow along in the action run; the reply will link the full session transcript.

📜 Live transcript — updating every ~5 minutes while the task runs.

🤖 On it — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/321); the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-94/run-321.md) — updating every ~5 minutes while the task runs.

claude-bot referenced this issue from a commit

2026-08-20 21:19:14 +00:00

fix: reject RepoAction.None instead of widening it to a full write grant

claude-bot referenced this issue from a pull request that will close it,

2026-08-20 21:19:31 +00:00

Claude: changes for issue #94 #95

claude-bot commented

2026-08-20 21:19:32 +00:00

Author

Collaborator

Fixed and pushed to claude/issue-94.

What changed

AppendRepoActions (src/ATProtoNet/Auth/OAuth/AtProtoScopes.cs) treated RepoAction.None and RepoAction.All identically — both emitted no action= list. Since an omitted list means the full default set in the scope grammar, AtProtoScopes.Repo("app.bsky.feed.post", RepoAction.None) handed back repo:app.bsky.feed.post: a create + update + delete grant, the opposite of what the caller asked for.

RepoAction.None now throws ArgumentException (ParamName = actions), mirroring what Space(...) does for SpaceAction.None. The guard sits in the shared helper rather than in each entry point, so it covers Repo(string, RepoAction), Repo(IReadOnlyList<string>, RepoAction), and the one-element list that delegates between them — all three are pinned by tests. RepoAction.All and every partial combination are untouched.

The two Repo overloads gained <exception> tags and an actions parameter note, and the RepoAction.None enum member now says it is rejected. I also updated SpaceAction.None's summary: Space(...) has thrown for it since #90, but the member still read "No specific action", which is now the same trap for the next reader.

CHANGELOG gets a new ### Breaking changes section under [Unreleased] with the migration note — a caller passing RepoAction.None was silently requesting full write access, so the fix is to drop the repo: scope entirely or name the narrowest action actually needed.

Verified

dotnet build src/ATProtoNet/ATProtoNet.csproj — 0 warnings, 0 errors.
dotnet test tests/ATProtoNet.Tests/ — 1514 passed, 0 failed, 5 skipped (the pre-existing Aspire manifest skips).

A full-solution dotnet build fails in this container on samples/ManagedPdsSample.AppHost, which can't resolve the Aspire.AppHost.Sdk package from nuget.org — a sandbox network limitation unrelated to this change, and not a project my diff touches. The canonical pre-merge check (the unit test project) runs clean.

⏱ 2m 55s · 21 turns · tokens: 41.4k in / 9.0k out (+912.6k cache-read)
📜 Full session transcript
→ Opened PR: #95

Fixed and pushed to `claude/issue-94`. ## What changed `AppendRepoActions` (`src/ATProtoNet/Auth/OAuth/AtProtoScopes.cs`) treated `RepoAction.None` and `RepoAction.All` identically — both emitted no `action=` list. Since an omitted list means the full default set in the scope grammar, `AtProtoScopes.Repo("app.bsky.feed.post", RepoAction.None)` handed back `repo:app.bsky.feed.post`: a create + update + delete grant, the opposite of what the caller asked for. `RepoAction.None` now throws `ArgumentException` (`ParamName` = `actions`), mirroring what `Space(...)` does for `SpaceAction.None`. The guard sits in the shared helper rather than in each entry point, so it covers `Repo(string, RepoAction)`, `Repo(IReadOnlyList<string>, RepoAction)`, and the one-element list that delegates between them — all three are pinned by tests. `RepoAction.All` and every partial combination are untouched. The two `Repo` overloads gained `<exception>` tags and an `actions` parameter note, and the `RepoAction.None` enum member now says it is rejected. I also updated `SpaceAction.None`'s summary: `Space(...)` has thrown for it since #90, but the member still read "No specific action", which is now the same trap for the next reader. CHANGELOG gets a new `### Breaking changes` section under `[Unreleased]` with the migration note — a caller passing `RepoAction.None` was silently requesting full write access, so the fix is to drop the `repo:` scope entirely or name the narrowest action actually needed. ## Verified - `dotnet build src/ATProtoNet/ATProtoNet.csproj` — 0 warnings, 0 errors. - `dotnet test tests/ATProtoNet.Tests/` — **1514 passed, 0 failed**, 5 skipped (the pre-existing Aspire manifest skips). A full-solution `dotnet build` fails in this container on `samples/ManagedPdsSample.AppHost`, which can't resolve the `Aspire.AppHost.Sdk` package from nuget.org — a sandbox network limitation unrelated to this change, and not a project my diff touches. The canonical pre-merge check (the unit test project) runs clean. --- ⏱ 2m 55s · 21 turns · tokens: 41.4k in / 9.0k out (+912.6k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-94/run-321.md) → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/95