Grandiras/ATProto.NET

Fork 0

Pds: PdsSessionService DI registration uses an ephemeral signing key — all sessions invalidate on restart #37

New issue

Closed

opened 2026-06-10 22:46:51 +00:00 by Grandiras · 2 comments

Grandiras commented

2026-06-10 22:46:51 +00:00

Owner

AddAtProtoPds() registers PdsSessionService via plain DI:

services.AddSingleton<PdsSessionService>();

Its constructor is PdsSessionService(PdsOptions options, byte[]? signingKey = null) — DI can't supply byte[], so every process start generates a random HMAC signing key, which invalidates all access/refresh tokens on every restart/redeploy. For a production PDS that means all clients are silently logged out whenever the container restarts.

Workaround (what Updraft does): re-register after AddAtProtoPds so the last registration wins:

builder.Services.AddSingleton(sp =>
    new PdsSessionService(sp.GetRequiredService<PdsOptions>(), Convert.FromBase64String(jwtSecret)));

Suggestion: add PdsOptions.SessionSigningKey (base64) and have the default registration use it when set, with a startup warning when falling back to an ephemeral key.

`AddAtProtoPds()` registers `PdsSessionService` via plain DI: ```csharp services.AddSingleton<PdsSessionService>(); ``` Its constructor is `PdsSessionService(PdsOptions options, byte[]? signingKey = null)` — DI can't supply `byte[]`, so every process start generates a **random HMAC signing key**, which invalidates all access/refresh tokens on every restart/redeploy. For a production PDS that means all clients are silently logged out whenever the container restarts. Workaround (what Updraft does): re-register after `AddAtProtoPds` so the last registration wins: ```csharp builder.Services.AddSingleton(sp => new PdsSessionService(sp.GetRequiredService<PdsOptions>(), Convert.FromBase64String(jwtSecret))); ``` Suggestion: add `PdsOptions.SessionSigningKey` (base64) and have the default registration use it when set, with a startup warning when falling back to an ephemeral key.

claude-bot was assigned by Grandiras

2026-07-25 14:36:43 +00:00

claude-bot commented

2026-07-25 14:36:46 +00:00

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/52); 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-37/run-52.md) — updating every ~5 minutes while the task runs.

claude-bot referenced this issue from a commit

2026-07-25 14:43:54 +00:00

feat: add PdsOptions.SessionSigningKey for persistent PDS session tokens

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

2026-07-25 14:46:56 +00:00

Claude: changes for issue #37 #61

claude-bot commented

2026-07-25 14:55:21 +00:00

Collaborator

Fixed on branch claude/issue-37.

What changed

PdsOptions.SessionSigningKey (base64, string?) — the persisted HMAC-SHA256 key for session tokens. AddAtProtoPds() (both overloads) now builds PdsSessionService through a factory instead of registering it by type, so the key actually reaches the constructor that DI could never fill:

builder.Services.AddAtProtoPds(options =>
{
    options.Hostname = "my-pds.example.com";
    options.SessionSigningKey = builder.Configuration["Pds:SessionSigningKey"];
});

The Updraft workaround (re-registering the singleton after AddAtProtoPds) still works and still wins, since an explicit byte[] takes precedence over the option.

Fallback behaviour when unset — still an ephemeral random key, but now warned about, and the warning fires at host startup, not on the first login: a small internal IHostedService (registered via TryAddEnumerable, so repeated AddAtProtoPds calls don't stack) forces the singleton to be constructed during StartAsync. That also makes a malformed key fail fast — an unparsable base64 value throws InvalidOperationException naming the option rather than a bare FormatException at token-issue time. A key that decodes to fewer than 32 bytes is accepted (existing PDS JWT secrets may be shorter) but warned about.

New public API on PdsSessionService: GenerateSigningKey() (32 random bytes, base64), ResolveSigningKey(PdsOptions) (decode/validate, null when unset — an unset env var binding to "" counts as unset), UsesEphemeralSigningKey, SigningKeySize.

Also updated docs/pds.md (new Session signing key section under Configuration), the PdsSample program, and CHANGELOG.md under [Unreleased] → Added.

Verified

dotnet build — 0 errors, no new warnings in ATProtoNet.Pds.
dotnet test tests/ATProtoNet.Tests/ — 1048 passed, 0 failed (up from 1035; 14 new tests, 1 removed).

New tests cover: a configured key surviving a simulated restart (two service instances / two DI containers sharing the config validate each other's tokens), the ephemeral key not surviving, explicit byte[] precedence over the option, base64 validation, the startup warning being emitted by a real HostBuilder before any request, the short-key warning, no warning when configured, and single startup-check registration across duplicate AddAtProtoPds calls.

→ Opened PR: #61

⏱ 7m 12s · 46 turns · 📜 Full session transcript

(reply recovered from the transcript — the posting step in runs before v12.3 was broken by a shell-quoting bug)

Fixed on branch **`claude/issue-37`**. ## What changed **`PdsOptions.SessionSigningKey`** (base64, `string?`) — the persisted HMAC-SHA256 key for session tokens. `AddAtProtoPds()` (both overloads) now builds `PdsSessionService` through a factory instead of registering it by type, so the key actually reaches the constructor that DI could never fill: ```csharp builder.Services.AddAtProtoPds(options => { options.Hostname = "my-pds.example.com"; options.SessionSigningKey = builder.Configuration["Pds:SessionSigningKey"]; }); ``` The Updraft workaround (re-registering the singleton after `AddAtProtoPds`) still works and still wins, since an explicit `byte[]` takes precedence over the option. **Fallback behaviour when unset** — still an ephemeral random key, but now warned about, and the warning fires at *host startup*, not on the first login: a small internal `IHostedService` (registered via `TryAddEnumerable`, so repeated `AddAtProtoPds` calls don't stack) forces the singleton to be constructed during `StartAsync`. That also makes a malformed key fail fast — an unparsable base64 value throws `InvalidOperationException` naming the option rather than a bare `FormatException` at token-issue time. A key that decodes to fewer than 32 bytes is accepted (existing PDS JWT secrets may be shorter) but warned about. **New public API on `PdsSessionService`:** `GenerateSigningKey()` (32 random bytes, base64), `ResolveSigningKey(PdsOptions)` (decode/validate, `null` when unset — an unset env var binding to `""` counts as unset), `UsesEphemeralSigningKey`, `SigningKeySize`. Also updated `docs/pds.md` (new *Session signing key* section under Configuration), the `PdsSample` program, and `CHANGELOG.md` under `[Unreleased] → Added`. ## Verified - `dotnet build` — 0 errors, no new warnings in `ATProtoNet.Pds`. - `dotnet test tests/ATProtoNet.Tests/` — **1048 passed, 0 failed** (up from 1035; 14 new tests, 1 removed). New tests cover: a configured key surviving a simulated restart (two service instances / two DI containers sharing the config validate each other's tokens), the ephemeral key not surviving, explicit `byte[]` precedence over the option, base64 validation, the startup warning being emitted by a real `HostBuilder` before any request, the short-key warning, no warning when configured, and single startup-check registration across duplicate `AddAtProtoPds` calls. → Opened PR: https://git.grandiras.net/Grandiras/ATProto.NET/pulls/61 --- ⏱ 7m 12s · 46 turns · 📜 [Full session transcript](https://git.grandiras.net/Grandiras/claude-bot/src/branch/claude-logs/transcripts/Grandiras/ATProto.NET/issue-37/run-52.md) _(reply recovered from the transcript — the posting step in runs before v12.3 was broken by a shell-quoting bug)_

Grandiras closed this issue

2026-07-25 14:59:22 +00:00

Grandiras referenced this issue from a commit

2026-07-25 14:59:22 +00:00

Claude: changes for issue #37 (#61)

claude-bot referenced this issue from a commit

2026-07-25 15:38:49 +00:00

Merge main into claude/issue-40

claude-bot referenced this issue

2026-07-25 15:39:20 +00:00

Claude: changes for issue #40 #65