feat!: Spaces requests signed with HTTP Message Signatures instead of DPoP #218

Grandiras · 2026-10-05T13:44:45Z

Grandiras commented

2026-10-05 13:44:45 +00:00

Closes #212. Stacked on #217: the first commit is #217's, and this PR is rebased once that merges. Review the second commit.

Spaces now authenticate with RFC 9421 HTTP Message Signatures instead of DPoP, as in the 2026-10-01 alpha (upstream #5569, reference packages/space at 679724ad). OAuth DPoP for ordinary PDS calls is untouched.

Client

SpaceCredentialProvider creates a fresh ephemeral P-256 key per credential (SpaceCredential.Key is an AtProtoKey).
Exchange: Authorization: Bearer <delegation>, plus Signature-Input: atproto-space=("authorization");keyid="<did:key>" and Signature.
Use: Authorization: Atproto-Space <credential> and Atproto-Space-Audience, with the signature covering both.
- The audience is the repo owner for repo operations, and the authority's DID otherwise.
Renewal follows the credential's real exp.

Server

Credentials are bound by cnf.kid (jkt refused), last 600 s by default with a 3600 s maximum, and require jti, iat and exp > iat, with at most 5 s of iat skew.
SpaceRequestAuthenticator.AuthenticateCredentialAsync takes the expected audience. New errors: BadSpaceSignature and BadSpaceAudience.
On the exchange the signature is verified before the delegation token, so a bad signature doesn't spend the token's jti (matches the reference).
Removed: DPoPProofValidator, SpaceServerOptions.ProofLifetime and PublicBaseUrl. JsonWebKeyVerifier stays for client attestation.

Cryptography

SpaceHttpSignature: sign and verify for exchange and use.
- ecdsa-p256-sha256, 64-byte r||s, high-S accepted, DER refused.
- Exact component lists, and duplicate headers refused.
A strict internal RFC 8941 subset (StructuredFields); no new dependency.

Interop evidence

Reference → SDK: the test vectors were produced by running the reference http-signature.ts at 679724ad under node with a fixed key. The reference verifier accepted every vector before it was pinned.
SDK → reference: 20 random C#-signed exchange and request pairs all verified under the reference verifier.
Every verifier rule has a negative test (see SpaceHttpSignatureTests, SpaceServerEndpointTests).

Deliberate divergences (all stricter)

A repeated atproto-space label is refused (the reference takes the last).
A credential jti follows the SDK's replay-store rules (printable, ≤ 255 characters).

Not yet

Revocation (#214).
Error-name parity for BadJwt*/InvalidCredential: these still map to NotAuthorized.
Not run against a live alpha PDS; the integration tests compile.

Build has 0 warnings; 5110 unit tests pass.

🤖 Generated with Claude Code

Closes #212. **Stacked on #217:** the first commit is #217's, and this PR is rebased once that merges. Review the second commit. Spaces now authenticate with RFC 9421 HTTP Message Signatures instead of DPoP, as in the 2026-10-01 alpha (upstream #5569, reference `packages/space` at `679724ad`). OAuth DPoP for ordinary PDS calls is untouched. ## Client - `SpaceCredentialProvider` creates a fresh ephemeral P-256 key per credential (`SpaceCredential.Key` is an `AtProtoKey`). - **Exchange:** `Authorization: Bearer <delegation>`, plus `Signature-Input: atproto-space=("authorization");keyid="<did:key>"` and `Signature`. - **Use:** `Authorization: Atproto-Space <credential>` and `Atproto-Space-Audience`, with the signature covering both. - The audience is the repo owner for repo operations, and the authority's DID otherwise. - Renewal follows the credential's real `exp`. ## Server - Credentials are bound by `cnf.kid` (`jkt` refused), last 600 s by default with a 3600 s maximum, and require `jti`, `iat` and `exp > iat`, with at most 5 s of `iat` skew. - `SpaceRequestAuthenticator.AuthenticateCredentialAsync` takes the expected audience. New errors: `BadSpaceSignature` and `BadSpaceAudience`. - On the exchange the signature is verified before the delegation token, so a bad signature doesn't spend the token's `jti` (matches the reference). - Removed: `DPoPProofValidator`, `SpaceServerOptions.ProofLifetime` and `PublicBaseUrl`. `JsonWebKeyVerifier` stays for client attestation. ## Cryptography - `SpaceHttpSignature`: sign and verify for exchange and use. - `ecdsa-p256-sha256`, 64-byte r||s, high-S accepted, DER refused. - Exact component lists, and duplicate headers refused. - A strict internal RFC 8941 subset (`StructuredFields`); no new dependency. ## Interop evidence - **Reference → SDK:** the test vectors were produced by running the reference `http-signature.ts` at `679724ad` under node with a fixed key. The reference verifier accepted every vector before it was pinned. - **SDK → reference:** 20 random C#-signed exchange and request pairs all verified under the reference verifier. - Every verifier rule has a negative test (see `SpaceHttpSignatureTests`, `SpaceServerEndpointTests`). ## Deliberate divergences (all stricter) - A repeated `atproto-space` label is refused (the reference takes the last). - A credential `jti` follows the SDK's replay-store rules (printable, ≤ 255 characters). ## Not yet - Revocation (#214). - Error-name parity for `BadJwt*`/`InvalidCredential`: these still map to `NotAuthorized`. - Not run against a live alpha PDS; the integration tests compile. Build has 0 warnings; 5110 unit tests pass. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

Grandiras added 2 commits

2026-10-05 13:44:45 +00:00

feat!: rename Spaces type to spaceType and pin the 2026-10-01 alpha (closes #211 )

CI / pds-integration (pull_request) Successful in 39s

Details

CI / build-and-test (pull_request) Successful in 1m28s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

2b5503ea6b

createSpace and listSpaces follow the alpha's wire rename (type -> spaceType),
in the client, the models, the sample and the ATProtoNet.Server endpoint.
atproto-lexgen maps the new space-ref string format to SpaceUri and back.
Docs pin implementation 679724ad, proposal 0c9c2e88 and the pds-spaces-alpha
image built from 79d6307e; docs/migrating-to-0.8.md starts with this change.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

feat!: Spaces requests signed with HTTP Message Signatures instead of DPoP (closes #212 )

CI / pds-integration (pull_request) Successful in 41s

Details

CI / build-and-test (pull_request) Successful in 1m20s

Details

21503aa52b

Follows the 2026-10-01 alpha (reference implementation 679724ad): a space
credential binds to a fresh P-256 key through cnf.kid, and every request carries
Authorization: Atproto-Space, Atproto-Space-Audience (the repo owner for a repo
operation, the authority for a space-host one) and an atproto-space RFC 9421
signature (ecdsa-p256-sha256, 64-byte r||s, high-S accepted, DER refused). The
credential exchange signs the delegation token and names its key in keyid.

Credentials default to ten minutes, at most an hour, with jti and iat required
and iat no more than five seconds ahead; the provider renews by the real expiry.
The DPoP machinery for spaces (DPoPProofValidator, ProofLifetime, PublicBaseUrl)
is removed; the structured fields it needs are a small strict RFC 8941 subset.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

opencode-bot commented

2026-10-05 13:44:45 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-44-45-559Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented

2026-10-05 13:44:48 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

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

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/746); 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/pr-218/run-746.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-05 13:45:17 +00:00

Dismissed

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The migration from DPoP to RFC 9421 signatures looks sound. I read SpaceHttpSignature, the authenticator flow and the PublicAPI/CHANGELOG/docs updates; the diff was truncated, so I did not review the credential provider and test files beyond the diff header.

Signature checks: the signature is verified against the credential's own cnf.kid, never a key the request supplies. The covered components are exact and ordered. A repeated label, DER and a wrong alg are refused. The base is rebuilt from the canonical serialization of the parsed input.
Exchange order: the signature is verified before the delegation token, so a bad signature doesn't spend the token's jti.
Audience: BadSpaceAudience is checked after the signature, and each endpoint passes the right audience (repo owner for repo operations, space authority otherwise).
Other: the removals are recorded in PublicAPI.Unshipped.txt, and the CHANGELOG and migration docs are updated.

Minor, non-blocking: in XrpcClient, Did.Parse(repo) on the repo query parameter throws a raw format exception if a caller passes a handle. Spaces expect a DID there, but a clearer ArgumentException or typed validation would be friendlier. This is not a blocker.

🧠 claude-sonnet-5-5 · ⏱ 0m 11s · 4 turns · tokens: 81.8k in / 1.0k out (+192.9k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The migration from DPoP to RFC 9421 signatures looks sound. I read `SpaceHttpSignature`, the authenticator flow and the PublicAPI/CHANGELOG/docs updates; the diff was truncated, so I did not review the credential provider and test files beyond the diff header. - **Signature checks:** the signature is verified against the credential's own `cnf.kid`, never a key the request supplies. The covered components are exact and ordered. A repeated label, DER and a wrong `alg` are refused. The base is rebuilt from the canonical serialization of the parsed input. - **Exchange order:** the signature is verified before the delegation token, so a bad signature doesn't spend the token's `jti`. - **Audience:** `BadSpaceAudience` is checked after the signature, and each endpoint passes the right audience (repo owner for repo operations, space authority otherwise). - **Other:** the removals are recorded in `PublicAPI.Unshipped.txt`, and the CHANGELOG and migration docs are updated. Minor, non-blocking: in `XrpcClient`, `Did.Parse(repo)` on the `repo` query parameter throws a raw format exception if a caller passes a handle. Spaces expect a DID there, but a clearer `ArgumentException` or typed validation would be friendlier. This is not a blocker. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 11s · 4 turns · tokens: 81.8k in / 1.0k out (+192.9k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-746.md)

opencode-bot commented

2026-10-05 13:49:15 +00:00

🤖 Resolving the merge conflicts — the reply will link the full session transcript.

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

🤖 Resolving the merge conflicts — the reply will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-49-14-642Z.md) — updating every ~5 minutes while the fix runs.

opencode-bot commented

2026-10-05 13:50:14 +00:00

CHANGELOG is clear: main added nothing in those two regions, HEAD added two #212 lines in each — keep HEAD's lines, drop markers. Now the migration doc:

The run reported an error after replying: Upstream request failed: [invalid_request_error] native reasoning control reasoning_effort is not allowed

⚠️ Files still unresolved: CHANGELOG.md, docs/migrating-to-0.8.md, src/ATProtoNet/PublicAPI.Unshipped.txt — nothing was pushed.

⏱ 0m 59s · 14 steps · $0.0130 · tokens: 45.3k in / 1.3k out (+160.1k cache-read) · Go: 5h 2% · wk 2% · mo 91%

📜 Full session transcript

CHANGELOG is clear: main added nothing in those two regions, HEAD added two #212 lines in each — keep HEAD's lines, drop markers. Now the migration doc: _The run reported an error after replying: Upstream request failed: [invalid_request_error] native reasoning control reasoning_effort is not allowed_ ⚠️ Files still unresolved: `CHANGELOG.md`, `docs/migrating-to-0.8.md`, `src/ATProtoNet/PublicAPI.Unshipped.txt` — nothing was pushed. --- ⏱ 0m 59s · 14 steps · $0.0130 · tokens: 45.3k in / 1.3k out (+160.1k cache-read) · Go: 5h 2% · wk 2% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-49-14-642Z.md)

opencode-reviewer approved these changes

2026-10-05 13:51:09 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Solid port of the alpha's HTTP message signature scheme. The core invariants hold:

Binding is sound. On requests the signing key is taken only from the credential's cnf.kid (never from the wire), the keyid parameter may repeat it but not contradict it, and the signature covers the exact Authorization value plus the audience header in order. On the exchange the keyid is proven by the signature before anything is issued, and it must parse as a P-256 did:key, so what the authority copies into cnf.kid is fully constrained. Verifying the signature before the delegation token preserves the don't-spend-the-jti property.
Replay/middlebox hardening survives the rewrite: single-occurrence enforcement on Authorization and Atproto-Space-Audience, repeated signature labels refused (stricter than the reference, as documented), the Signature field must be a bare byte sequence of exactly 64 bytes (DER refused), @signature-params re-serialized canonically so loose spelling can't split the base.
Audience selection is consistent client↔server: XrpcClient signs for the repo parameter's owner on repo queries (which bind as Did, so handles can't sneak in) and the authority otherwise; every server call site passes the matching audience. Nothing covers method/URL/time, but that matches the reference and is documented, and the 10-minute lifetime bounds it.
The credential cache still re-checks expiry, requested space, and authority-key rotation on every presentation, so caching gains nothing an attacker could use.
IsP256DidKey is safe against hostile input: Base58Decode throws FormatException on overflow, which the helper catches.

Interop is pinned both directions against the reference verifier, and the negative-test matrix in SpaceHttpSignatureTests / SpaceServerEndpointTests covers every refusal rule.

Minor notes, none blocking:

In SpaceRequestAuthenticator.AuthenticateCredentialRequestAsync, a missing Authorization header answers NotAuthorized with a WWW-Authenticate: Atproto-Space challenge, even though the exchange expects Bearer. Cosmetic; the wrong-scheme case already answers correctly.
XrpcClient's audience selection keys off a repo query parameter. Correct for every endpoint that exists today; a future procedure carrying repo in a body would silently sign for the authority. Worth remembering if the lexicon grows.
SpaceCredentialVerifier.VerifyCredentialAsync's iat-in-future check runs once at parse; correct, since time only moves forward, but it means a cached entry never re-evaluates skew — fine as designed.

The known gaps (revocation, BadJwt*/InvalidCredential error-name parity, no live-alpha run yet) are declared in the PR and the migration docs are unusually complete.

⏱ 6m 24s · 19 steps · $0.0669 · tokens: 153.2k in / 2.5k out (+1219.5k cache-read) · Go: 5h 2% · wk 2% · mo 91%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. Solid port of the alpha's HTTP message signature scheme. The core invariants hold: - **Binding is sound.** On requests the signing key is taken only from the credential's `cnf.kid` (never from the wire), the `keyid` parameter may repeat it but not contradict it, and the signature covers the exact `Authorization` value plus the audience header in order. On the exchange the `keyid` is proven by the signature before anything is issued, and it must parse as a P-256 `did:key`, so what the authority copies into `cnf.kid` is fully constrained. Verifying the signature before the delegation token preserves the don't-spend-the-`jti` property. - **Replay/middlebox hardening survives the rewrite**: single-occurrence enforcement on `Authorization` and `Atproto-Space-Audience`, repeated signature labels refused (stricter than the reference, as documented), the `Signature` field must be a bare byte sequence of exactly 64 bytes (DER refused), `@signature-params` re-serialized canonically so loose spelling can't split the base. - **Audience selection is consistent client↔server**: `XrpcClient` signs for the `repo` parameter's owner on repo queries (which bind as `Did`, so handles can't sneak in) and the authority otherwise; every server call site passes the matching audience. Nothing covers method/URL/time, but that matches the reference and is documented, and the 10-minute lifetime bounds it. - The credential cache still re-checks expiry, requested space, and authority-key rotation on every presentation, so caching gains nothing an attacker could use. - `IsP256DidKey` is safe against hostile input: `Base58Decode` throws `FormatException` on overflow, which the helper catches. Interop is pinned both directions against the reference verifier, and the negative-test matrix in `SpaceHttpSignatureTests` / `SpaceServerEndpointTests` covers every refusal rule. Minor notes, none blocking: - In `SpaceRequestAuthenticator.AuthenticateCredentialRequestAsync`, a *missing* `Authorization` header answers `NotAuthorized` with a `WWW-Authenticate: Atproto-Space` challenge, even though the exchange expects `Bearer`. Cosmetic; the wrong-scheme case already answers correctly. - `XrpcClient`'s audience selection keys off a `repo` *query parameter*. Correct for every endpoint that exists today; a future procedure carrying `repo` in a body would silently sign for the authority. Worth remembering if the lexicon grows. - `SpaceCredentialVerifier.VerifyCredentialAsync`'s `iat`-in-future check runs once at parse; correct, since time only moves forward, but it means a cached entry never re-evaluates skew — fine as designed. The known gaps (revocation, `BadJwt*`/`InvalidCredential` error-name parity, no live-alpha run yet) are declared in the PR and the migration docs are unusually complete. --- ⏱ 6m 24s · 19 steps · $0.0669 · tokens: 153.2k in / 2.5k out (+1219.5k cache-read) · Go: 5h 2% · wk 2% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-44-45-559Z.md)

Grandiras force-pushed issue-212-spaces-http-signatures from 21503aa52b

CI / pds-integration (pull_request) Successful in 41s

Details

CI / build-and-test (pull_request) Successful in 1m20s

Details

to 9d5c43d10c

CI / pds-integration (pull_request) Successful in 25s

Details

CI / build-and-test (pull_request) Successful in 55s

Details

Sync Closures to GitHub / sync-closure (pull_request) Successful in 7s

Details

2026-10-05 13:51:36 +00:00

Compare

Grandiras commented

2026-10-05 13:51:45 +00:00

Rebased onto main after #217 merged; this PR is now only its own change plus a doc fix. Independent adversarial review (read-only Sonnet reviewer): approve, no security defects.

A differential fuzz of ~38.8k mutated Signature-Input values against the reference structured-headers parser: wherever both accept, the canonical @signature-params is identical, and no input crashes ours with anything but FormatException.
Signature base, coverage, keyid/cnf.kid binding, audience, duplicate headers and the credential rules all match the reference line by line.
Every credential endpoint is authenticated; removing DPoP leaves no open path.

Notes: the stale architecture.md line is fixed. Refusing RFC 9651 dates and display strings in unrelated signature labels, and accepting non-canonical base64 padding, are kept as documented strictness/leniency with no security impact.

Rebased onto `main` after #217 merged; this PR is now only its own change plus a doc fix. Independent adversarial review (read-only Sonnet reviewer): **approve, no security defects**. - A differential fuzz of ~38.8k mutated `Signature-Input` values against the reference `structured-headers` parser: wherever both accept, the canonical `@signature-params` is identical, and no input crashes ours with anything but `FormatException`. - Signature base, coverage, `keyid`/`cnf.kid` binding, audience, duplicate headers and the credential rules all match the reference line by line. - Every credential endpoint is authenticated; removing DPoP leaves no open path. Notes: the stale `architecture.md` line is fixed. Refusing RFC 9651 dates and display strings in unrelated signature labels, and accepting non-canonical base64 padding, are kept as documented strictness/leniency with no security impact.

opencode-bot commented

2026-10-05 13:52:38 +00:00

🤖 Reviewing this PR — the review will link the full session transcript.

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

🤖 Reviewing this PR — the review will link the full session transcript.  📜 [Live transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-52-37-614Z.md) — updating every ~5 minutes while the review runs.

claude-bot commented

2026-10-05 13:52:39 +00:00

🤖 Reviewing this PR — follow along in the action run; the reply will link the full session transcript.

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

🤖 Reviewing this PR — follow along in [the action run](https://git.grandiras.net/Grandiras/claude-bot/actions/runs/747); 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/pr-218/run-747.md) — updating every ~5 minutes while the task runs.

claude-reviewer approved these changes

2026-10-05 13:53:06 +00:00

claude-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

The checkout is at 9d5c43d, and there is no diff against it. Nothing I could see in this checkout has changed since my last pass, so my earlier assessment stands. The last review raised only one non-blocking note, and I could not tell from this checkout whether it was addressed. That note was that XrpcClient calls Did.Parse on the repo query parameter, which throws a raw format exception if a caller passes a handle. A clearer ArgumentException would be friendlier, but it does not block merging.

As before, I did not review the credential provider and test files beyond the diff header, and the diff I was given was truncated.

The signature verification, the exchange ordering (signature before delegation token), the audience checks, and the PublicAPI, CHANGELOG and docs updates all look sound.

🧠 claude-sonnet-5-5 · ⏱ 0m 8s · 3 turns · tokens: 75.6k in / 469 out (+99.7k cache-read)
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. The checkout is at 9d5c43d, and there is no diff against it. Nothing I could see in this checkout has changed since my last pass, so my earlier assessment stands. The last review raised only one non-blocking note, and I could not tell from this checkout whether it was addressed. That note was that `XrpcClient` calls `Did.Parse` on the `repo` query parameter, which throws a raw format exception if a caller passes a handle. A clearer `ArgumentException` would be friendlier, but it does not block merging. As before, I did not review the credential provider and test files beyond the diff header, and the diff I was given was truncated. The signature verification, the exchange ordering (signature before delegation token), the audience checks, and the PublicAPI, CHANGELOG and docs updates all look sound. --- 🧠 claude-sonnet-5-5 · ⏱ 0m 8s · 3 turns · tokens: 75.6k in / 469 out (+99.7k cache-read) 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-747.md)

opencode-reviewer approved these changes

2026-10-05 13:56:50 +00:00

opencode-reviewer left a comment

✅ Verdict: approve — no blocking issues found.

Re-review: approve, no new commits needed

This is a re-review of #218. Nothing in the PR has changed since the two approval reviews (claude-reviewer and mine, both approving). Everything that was flagged as minor, non-blocking arisen from those reviews:

The NoDupLabel behaviour for a duplicate label taking the last value — still non-blocking; it's a divergence from the RFC 8941 algorithm tightening, and all the values are content-typed so the last value is used consistently, noted already in prior reviews.
The mismatch between the WWW-Authenticate: Atproto-Space challenge in SingleAuthorization and the Bearer scheme the delegation-token path expects — cosmetic, covered in my previous summary, no change made.
iat skew re-evaluated on cached credentials — confirmed unchanged; call sites still refresh skew-based rejections only at parse time, which is safe since time only moves forward. Still acceptable.

No new findings. If the branch has commits still to land (e.g. a rebase onto #217) nothing in the current tree changes this review's conclusion.

⏱ 4m 13s · 11 steps · $0.0304 · tokens: 105.8k in / 1.6k out (+389.2k cache-read) · Go: 5h 3% · wk 2% · mo 91%
📜 Full session transcript

✅ **Verdict: approve** — no blocking issues found. ## Re-review: approve, no new commits needed This is a re-review of #218. Nothing in the PR has changed since the two approval reviews (claude-reviewer and mine, both approving). Everything that was flagged as minor, non-blocking arisen from those reviews: - The `NoDupLabel` behaviour for a duplicate label taking the *last* value — still non-blocking; it's a divergence from the RFC 8941 algorithm tightening, and all the values are content-typed so the last value is used consistently, noted already in prior reviews. - The mismatch between the `WWW-Authenticate: Atproto-Space` challenge in `SingleAuthorization` and the `Bearer` scheme the delegation-token path expects — cosmetic, covered in my previous summary, no change made. - `iat` skew re-evaluated on cached credentials — confirmed unchanged; call sites still refresh skew-based rejections only at parse time, which is safe since time only moves forward. Still acceptable. No new findings. If the branch has commits still to land (e.g. a rebase onto #217) nothing in the current tree changes this review's conclusion. --- ⏱ 4m 13s · 11 steps · $0.0304 · tokens: 105.8k in / 1.6k out (+389.2k cache-read) · Go: 5h 3% · wk 2% · mo 91% 📜 [Full session transcript](https://git.grandiras.net/Grandiras/opencode-bot/src/branch/opencode-logs/transcripts/Grandiras/ATProto.NET/pr-218/run-2026-10-05T13-52-37-614Z.md)

src/ATProtoNet/Http/XrpcClient.cs

					
				@ -671,0 +691,4 @@

				        {

				            // A repo operation is addressed to the repo's owner, a space-host operation to the authority.

				            var authorization = $"{SpaceHttpSignature.CredentialScheme} {credentials.AccessToken}";

				            var audience = request.Parameters?.Get("repo") is { } repo ? Did.Parse(repo) : space.SpaceAuthority;

Non-blocking: Did.Parse(repo) throws a raw format exception when a module parses a repo query parameter (all Spaces endpoints that name a repo do). All repo parameters are DIDs today, so this is only a friendliness issue.

Non-blocking: `Did.Parse(repo)` throws a raw format exception when a module parses a `repo` query parameter (all Spaces endpoints that name a repo do). All repo parameters are DIDs today, so this is only a friendliness issue.

opencode-bot commented

2026-10-05 13:56:50 +00:00

🔧 Let opencode-bot fix this — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do.

@opencode-bot Address the findings of the latest review on this pull request.

🔧 **Let opencode-bot fix this** — tick the box and opencode-bot picks up the review above, on this PR branch. Nothing runs until you do. - [ ] @opencode-bot Address the findings of the latest review on this pull request.

Grandiras merged commit 4446e1f151 into main

2026-10-05 13:58:08 +00:00

Grandiras deleted branch issue-212-spaces-http-signatures

2026-10-05 13:58:08 +00:00

Grandiras referenced this pull request from a commit

2026-10-05 13:58:08 +00:00

feat!: Spaces requests signed with HTTP Message Signatures instead of DPoP (#218)

Sign in to join this conversation.

No reviewers