fix(assistant): single use under racing requests, a cap on registering, Caddy reloaded

A connect code, an authorization code and a refresh token are each
consumed by deleting their row and going on only if that deleted it, so
of two requests racing with the same one only one succeeds (as OAuth
requires); tests race them. Registration, open to anyone, takes at most
200 clients an hour (429 after), so a flood can't fill the table before
pruning. The deploy reloads Caddy in place when deploy/Caddyfile changed,
as `docker compose up` doesn't restart it for a file it mounts: without
it the connector's paths wouldn't reach the web server.

Also formats the app's fake assistant gateway, new and so unchecked by
the last commit's check (tool/check.sh formats tracked files only).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MMm7usAo6q9ZZRoPkWjoxi
Signed-off-by: Tizian 「ティツィアーン」 <raisondetredev@pm.me>
This commit is contained in:
Tizian 「ティツィアーン」 2026-10-10 12:20:47 +02:00
commit 9f1716f16d
Signed by: raisondetredev
GPG key ID: 07F6FCF405488B64
4 changed files with 136 additions and 9 deletions

View file

@ -67,6 +67,12 @@ main() {
# schema; migrations that only add, as they should, keep that harmless.) # schema; migrations that only add, as they should, keep that harmless.)
docker compose run --rm --no-deps -e role=maintenance server --apply-migrations docker compose run --rm --no-deps -e role=maintenance server --apply-migrations
docker compose up -d --remove-orphans docker compose up -d --remove-orphans
# Caddy reads its Caddyfile when it starts, and `up` doesn't restart it for a
# changed file it mounts: a changed one is reloaded in place, without dropping
# connections.
if ! git diff --quiet "$deployed" "$commit" -- Caddyfile; then
docker compose exec -T caddy caddy reload --config /etc/caddy/Caddyfile
fi
wait_until_answering wait_until_answering
trap - EXIT trap - EXIT
docker image prune -f >/dev/null docker image prune -f >/dev/null

View file

@ -6,6 +6,7 @@ import 'package:halkyon_learn_flutter/src/feature/assistant/model/connected_assi
class FakeAssistantGateway({ class FakeAssistantGateway({
/// Where the connector is; none when null. /// Where the connector is; none when null.
final String? url = 'https://learn.example.org/mcp', final String? url = 'https://learn.example.org/mcp',
/// The assistants connected at first. /// The assistants connected at first.
final List<ConnectedAssistant> initially = const [], final List<ConnectedAssistant> initially = const [],
}) implements AssistantGateway { }) implements AssistantGateway {

View file

@ -25,6 +25,12 @@ const unconnectedClientLifetime = Duration(days: 7);
/// The one scope: reading the person's content. /// The one scope: reading the person's content.
const readScope = 'studysets:read'; const readScope = 'studysets:read';
/// How many clients may register in an hour: registration is open to
/// anyone, so this keeps a flood from filling the table before pruning
/// (clients nobody connects go after a week). Claude registers once per
/// person adding the connector.
const maxRegistrationsPerHour = 200;
/// The OAuth side of the assistant connector (ADR 0006): its metadata, client /// The OAuth side of the assistant connector (ADR 0006): its metadata, client
/// registration, the authorization page, the token endpoint, and checking /// registration, the authorization page, the token endpoint, and checking
/// a bearer token. [base] is where the web server is reached from outside, /// a bearer token. [base] is where the web server is reached from outside,
@ -123,6 +129,18 @@ class Connector({required final String base, required final String instance}) {
final String name when name.trim().isNotEmpty => _plain(name, 100), final String name when name.trim().isNotEmpty => _plain(name, 100),
_ => 'MCP client', _ => 'MCP client',
}; };
final hourAgo = clock.now().toUtc().subtract(const Duration(hours: 1));
if (await AssistantClient.db.count(
session,
where: (t) => t.createdAt > hourAgo,
) >=
maxRegistrationsPerHour) {
return ConnectorResponse.oauthError(
429,
'temporarily_unavailable',
'Too many registrations in the last hour; try again later.',
);
}
final secret = method == 'none' ? null : randomSecret(); final secret = method == 'none' ? null : randomSecret();
final client = await AssistantClient.db.insertRow( final client = await AssistantClient.db.insertRow(
session, session,
@ -247,13 +265,17 @@ class Connector({required final String base, required final String instance}) {
); );
} }
final grant = randomSecret(); final grant = randomSecret();
await session.db.transaction((transaction) async { final granted = await session.db.transaction((transaction) async {
// Used once. // Used once: of two requests racing with it, only the one that deletes
await AssistantConnectCode.db.deleteRow( // it goes on.
final spent = await AssistantConnectCode.db.deleteWhere(
session, session,
code, where: (t) => t.id.equals(code.id),
transaction: transaction, transaction: transaction,
); );
if (spent.isEmpty) {
return false;
}
// One connection per person and client: connecting again renews it. // One connection per person and client: connecting again renews it.
final connection = final connection =
await AssistantConnection.db.findFirstRow( await AssistantConnection.db.findFirstRow(
@ -285,7 +307,16 @@ class Connector({required final String base, required final String instance}) {
), ),
transaction: transaction, transaction: transaction,
); );
return true;
}); });
if (!granted) {
return ConnectorResponse(
200,
contentType: 'text/html; charset=utf-8',
body: authorizePage(page, wrongCode: true),
headers: pageHeaders(redirectUri),
);
}
return ConnectorResponse.redirect( return ConnectorResponse.redirect(
redirectUri.replace( redirectUri.replace(
queryParameters: { queryParameters: {
@ -334,11 +365,15 @@ class Connector({required final String base, required final String instance}) {
connection: AssistantConnection.include(), connection: AssistantConnection.include(),
), ),
); );
if (grant == null) { // Used once, whatever comes of it: of two requests racing with it,
// only the one that deletes it goes on.
if (grant == null ||
(await AssistantGrant.db.deleteWhere(
session,
where: (t) => t.id.equals(grant.id),
)).isEmpty) {
return _invalidGrant('The code is unknown or used.'); return _invalidGrant('The code is unknown or used.');
} }
// Used once, whatever comes of it.
await AssistantGrant.db.deleteRow(session, grant);
if (!grant.expiresAt.isAfter(now) || if (!grant.expiresAt.isAfter(now) ||
grant.connection?.clientId != client.id || grant.connection?.clientId != client.id ||
form['redirect_uri'] != grant.redirectUri || form['redirect_uri'] != grant.redirectUri ||
@ -365,8 +400,13 @@ class Connector({required final String base, required final String instance}) {
tokens.connection?.clientId != client.id) { tokens.connection?.clientId != client.id) {
return _invalidGrant('The refresh token is unknown or expired.'); return _invalidGrant('The refresh token is unknown or expired.');
} }
// Spent: a new pair replaces it. // Spent: a new pair replaces it, for the one request that deletes it.
await AssistantToken.db.deleteRow(session, tokens); if ((await AssistantToken.db.deleteWhere(
session,
where: (t) => t.id.equals(tokens.id),
)).isEmpty) {
return _invalidGrant('The refresh token is used.');
}
return await _issue(session, tokens.connectionId, tokens.scope, now); return await _issue(session, tokens.connectionId, tokens.scope, now);
default: default:
return ConnectorResponse.oauthError( return ConnectorResponse.oauthError(

View file

@ -272,6 +272,86 @@ void main() {
); );
}); });
test('at most 200 clients register in an hour', () async {
final now = DateTime.now().toUtc();
await AssistantClient.db.insert(session, [
for (var i = 0; i < maxRegistrationsPerHour; i++)
AssistantClient(
clientId: 'flood-$i',
name: 'Flood',
redirectUris: [_callback],
createdAt: now,
),
]);
final response = await connector.register(
session,
connectorRequest(
'POST',
body: jsonEncode({
'redirect_uris': [_callback],
}),
),
);
expect(response.status, 429);
});
group('Single use, also when requests race', () {
test('a connect code', () async {
final client = await register();
final code = await endpoints.assistant.connectCode(signedIn(alice));
final answers = await Future.wait([
typeCode(client, code.code),
typeCode(client, code.code),
]);
expect([for (final answer in answers) answer.status]..sort(), [
200,
302,
]);
expect(await AssistantGrant.db.count(session), 1);
});
test('an authorization code', () async {
final client = await register();
final code = await connect(alice, client);
final answers = await Future.wait([
exchange(codeForm(client, code)),
exchange(codeForm(client, code)),
]);
expect([for (final answer in answers) answer.status]..sort(), [
200,
400,
]);
expect(await AssistantToken.db.count(session), 1);
});
test('a refresh token', () async {
final client = await register();
final tokens = await tokensOf(alice, clientId: client);
final refresh = {
'grant_type': 'refresh_token',
'refresh_token': '${tokens['refresh_token']}',
'client_id': client,
};
final answers = await Future.wait([
exchange(refresh),
exchange(refresh),
]);
expect([for (final answer in answers) answer.status]..sort(), [
200,
400,
]);
expect(await AssistantToken.db.count(session), 1);
});
});
group('Connecting by code', () { group('Connecting by code', () {
test('a code needs a signed-in person', () async { test('a code needs a signed-in person', () async {
await expectLater( await expectLater(