Skip to content

Commit 6231b73

Browse files
JuliaEdomclaude
andcommitted
fix(auth): give a key configured without a key_id the same id on every load
A key whose configuration carries no key_id -- every key from AGENTFLOW_API_KEYS, and a key-file entry without one -- drew a random id from generate_key_id on every load. A key-file entry kept its id only when load() could write it back, and docker-compose.prod.yml mounts the key file read-only, over a config/api_keys.yaml whose two entries carry no key_id. The id names the key's Redis bucket, its api_usage rows and the admin views keyed by id: replicas sharing Redis each kept their own bucket for the same key (N replicas, N times its rpm), and every restart and reload started a fresh bucket and split its usage history. ensure_key_ids now derives the id -- <tenant>-<name>-<8 hex of the entry's peppered lookup digest>, from its stored key_lookup or compute_key_lookup over its plaintext -- and _legacy_env_keys runs environment keys through it. A clash lengthens the digest prefix, so ids stay unique and stable. A new pepper changes only an id derived from plaintext and never written back -- every environment key, and a plaintext entry of a key file the process cannot write; an entry with a stored key_lookup, and a derived id already written to a writable key file, keep theirs. A legacy hash-only entry has nothing to derive from and keeps a random id; generate_key_id and create_key are unchanged. tests/unit/test_auth_key_identity.py takes each scenario of docs/specs/api-key-identity.md (nine of its twelve tests fail against the code before this change), with false-reject controls that load the shipped key file and a persisted id beside a hash-only entry. The new key_rotation helpers are killed from test_key_rotation_mutation.py, the file the mutation gate runs; its one test that pinned a random id for entries with a plaintext key now builds hash-only entries, the case that keeps one. The CHANGELOG's ten LF-only lines are now CRLF like the rest of the file. docs/architecture.md and the reload requirement of docs/specs/api-key-rate-limiting.md now name the one key a reload still cannot keep: a legacy hash-only entry in a key file the process cannot write, whose random id -- and so its bucket and window -- changes on every load. docs/specs/api-key-identity.md already specified that id. Reviewed by opus-cli, the writer's own engine: glm-5.3 was rate-limited until 2026-09-14 and codex until 2026-10-05. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HHUBaNNPWihtsgeFWnRmaS
1 parent 1582ed1 commit 6231b73

7 files changed

Lines changed: 480 additions & 24 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,28 @@ All notable changes to AgentFlow are documented in this file.
44

55
## [Unreleased]
66

7-
### Fixed — reloading the key store no longer resets rate-limit windows (T-46)
7+
### Fixed — a key configured without a key_id keeps the same id on every load
8+
9+
A key whose configuration carries no `key_id` — every key from
10+
`AGENTFLOW_API_KEYS`, and a key-file entry without one — drew a random id on
11+
every `AuthManager.load()`. A key-file entry kept its id only when the id could
12+
be written back, and `docker-compose.prod.yml` mounts the key file read-only.
13+
The id names the key's Redis bucket (`kid:<key_id>`), its `api_usage` rows and
14+
the admin views keyed by id, so replicas sharing Redis each kept their own
15+
bucket for the same key — N replicas, N times its rpm — and every restart and
16+
reload started a fresh bucket and split the key's usage history. Such a key now
17+
gets `<tenant>-<name>-<first 8 hex of its key-lookup digest>`: the stored
18+
`key_lookup`, else the peppered `compute_key_lookup` of its plaintext, never
19+
the plaintext or an unpeppered hash of it. The same key gets the same id on
20+
every load, restart and replica that shares the pepper. Changing the pepper
21+
changes only an id derived from a plaintext key and never written back: every
22+
environment key, and a plaintext key-file entry in a file the process cannot
23+
write. A writable key file keeps the id written to it on the first load, and an
24+
entry with a stored `key_lookup` keeps the id derived from that digest. An id
25+
already taken lengthens the digest prefix. A legacy hash-only entry, with
26+
neither a plaintext key nor a `key_lookup`, still gets a random id.
27+
28+
### Fixed — reloading the key store no longer resets rate-limit windows
829

930
`AuthManager.load()` carried the in-memory rate-limit windows over by the
1031
plaintext key index (`keys_by_value`), but since audit S-6 every window is
@@ -14,9 +35,12 @@ emptied them all. `is_rate_limited()` and the in-memory secondary check in
1435
`check_rate_limit()` then handed every tenant a fresh budget, which during a
1536
Redis outage is the whole limit. Windows are now carried over by bucket name:
1637
a still-configured key keeps its window, a removed key loses it, and no
17-
plaintext key names a window at any point of the reload. Keys from
18-
`AGENTFLOW_API_KEYS` still get a new random `key_id` on every load, so their
19-
windows keep resetting until T-47.
38+
plaintext key names a window at any point of the reload. A window survives
39+
only while its key keeps its `key_id`, which a key configured without one —
40+
from `AGENTFLOW_API_KEYS`, or in a key file the process cannot write — now does
41+
too (entry above), with one exception: a legacy hash-only entry, with neither a
42+
plaintext key nor a `key_lookup`, in a key file the process cannot write still
43+
gets a new id, and so an emptied window, on every reload.
2044

2145
### Docs — the pre-push hedges outlived the push
2246

@@ -605,16 +629,16 @@ applies `--ignore` only for `safety_id` values whose waiver scope matches that
605629
bucket. Expired waivers stop suppressing findings. A waiver whose scope is not
606630
scanned, or a duplicate `safety_id`, fails closed.
607631

608-
### Documentation — root records archived (2026-09-02–2026-09-04)
609-
610-
Twenty-one immutable tracked Markdown records moved from the repository root
611-
to `docs/evidence/records/` with unchanged filenames and SHA-256 digests.
612-
Living citations now use those paths. After all eight documentation-cleanup
613-
items closed, `plan_26_08_2026.md` moved to `docs/archive/plans/` with its
614-
closure evidence intact. The retired local 2026-04-17 benchmark baseline moved
615-
from the standalone `docs/benchmark-baseline-archive/` directory into the
616-
performance archive.
617-
632+
### Documentation — root records archived (2026-09-02–2026-09-04)
633+
634+
Twenty-one immutable tracked Markdown records moved from the repository root
635+
to `docs/evidence/records/` with unchanged filenames and SHA-256 digests.
636+
Living citations now use those paths. After all eight documentation-cleanup
637+
items closed, `plan_26_08_2026.md` moved to `docs/archive/plans/` with its
638+
closure evidence intact. The retired local 2026-04-17 benchmark baseline moved
639+
from the standalone `docs/benchmark-baseline-archive/` directory into the
640+
performance archive.
641+
618642
### Security — nltk 3.10.0 -> 3.10.3 in uv.lock (Dependabot GHSA-m4rf-3fr8-xwx3, GHSA-6hwm-xvph-95vm)
619643

620644
- `uv lock --upgrade-package nltk` only; nltk is a transitive dependency of `llama-index-core` and is not part of the `cloud`/`postgres` export, so `requirements-docker.lock` is unchanged. Closes the critical (JVM argument injection in the Stanford wrappers) and high (uncontrolled `dot` search path) advisories GitHub reported on the default branch on 2026-09-01.

‎docs/architecture.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ See [Architecture Decision Records](decisions/) for detailed trade-off analysis.
134134
## Security
135135

136136
### Implemented
137-
- **API authentication**: API key via `X-API-Key` header (set `AGENTFLOW_API_KEYS` env var). Requirements for a key's id: [API key identity](specs/api-key-identity.md)
137+
- **API authentication**: API key via `X-API-Key` header (set `AGENTFLOW_API_KEYS` env var). A key configured without a `key_id` (every environment key, and a key-file entry without one) gets an id derived from its key-lookup digest, stable across reloads, restarts and replicas (except a legacy hash-only entry, with neither a plaintext key nor a `key_lookup`: it has nothing to derive from and keeps a random id, which in a key file the process cannot write changes on every load); changing the pepper changes only an id derived from a plaintext key and never written back (every environment key, and a plaintext key-file entry in a file the process cannot write), while a writable key file keeps the id written to it on the first load and an entry with a stored `key_lookup` keeps the id derived from that digest. Requirements for a key's id: [API key identity](specs/api-key-identity.md)
138138
- **Transport gate (audit P2-3)**: `AGENTFLOW_PROFILE=production` refuses to boot over plaintext transport to an external ClickHouse/Redis/PostgreSQL (loopback exempt; deliberate exceptions named in `AGENTFLOW_INSECURE_TRANSPORT_OK`), and refuses a wildcard CORS origin outside demo mode. The ClickHouse client supports HTTPS with hostname verification and a private-CA bundle (`CLICKHOUSE_SECURE`, `CLICKHOUSE_CA_CERT`)
139139
- **Rate limiting**: Per-key sliding window with Redis backing when available and in-memory fallback for local/test, configurable via `AGENTFLOW_RATE_LIMIT_RPM` (default: 120/min). Requirements: [API key rate limiting](specs/api-key-rate-limiting.md)
140140
- **Health/docs exempt**: `/v1/health`, `/docs`, `/metrics` don't require auth

‎docs/specs/api-key-rate-limiting.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ id, and no bucket name SHALL contain a plaintext API key.
2020
Reloading the key configuration — `load()`, a SIGHUP reload, or the reload that
2121
ends every key create, rotate and revoke — SHALL keep the in-memory rate-limit
2222
window of every key that is still configured, and SHALL drop the window of a
23-
key the reload removed.
23+
key the reload removed. A key whose id changes on every load, a legacy
24+
hash-only entry in a key file the process cannot write (see
25+
[API key identity](api-key-identity.md)), gets a new bucket on every reload, so
26+
its window starts empty.
2427

2528
### Scenario: a full window survives a reload
2629
- **GIVEN** a key with `rate_limit_rpm: 1` that has already made its one request in the current window

‎src/agentflow_runtime/serving/api/auth/key_rotation.py‎

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import re
55
import secrets
66
import threading
7+
from collections.abc import Collection
78
from datetime import UTC, datetime, timedelta
89

910
try:
@@ -22,6 +23,42 @@
2223
is_permission_denied,
2324
)
2425

26+
# A derived key_id ends in this many characters of the key's lookup digest: the
27+
# same shape as the 8 hex characters `generate_key_id` draws at random.
28+
KEY_ID_DIGEST_CHARS = 8
29+
30+
31+
def key_id_slug(value: str, fallback: str) -> str:
32+
return "-".join(re.findall(r"[a-z0-9]+", value.lower())) or fallback
33+
34+
35+
def derived_key_id(item: TenantKey, existing_ids: Collection[str]) -> str | None:
36+
"""A stable key_id for an entry configured without one, or None.
37+
38+
The suffix is a prefix of the entry's peppered lookup digest -- its stored
39+
`key_lookup`, else `compute_key_lookup` over its plaintext key -- so the
40+
same key gets the same id on every load, restart and replica that shares
41+
the pepper. Never the plaintext or an unpeppered hash of it: the id lands in
42+
logs, Redis key names, usage rows and admin responses, and must not let a
43+
guessed key be confirmed offline (audit FB-07).
44+
45+
An id already taken lengthens the prefix, so the later entry of a clashing
46+
pair still gets the same id on every load. None when there is nothing to
47+
derive from (a legacy hash-only entry) or the whole digest is taken.
48+
"""
49+
if item.key_lookup is not None:
50+
lookup = item.key_lookup
51+
elif item.key is not None:
52+
lookup = compute_key_lookup(item.key)
53+
else:
54+
return None
55+
stem = f"{key_id_slug(item.tenant, 'tenant')}-{key_id_slug(item.name, 'agent')}-"
56+
for size in range(KEY_ID_DIGEST_CHARS, len(lookup) + 1):
57+
candidate = stem + lookup[:size]
58+
if candidate not in existing_ids:
59+
return candidate
60+
return None
61+
2562

2663
class KeyRotator:
2764
def __init__(self, manager: AuthManager) -> None:
@@ -274,8 +311,8 @@ def generate_key_id(
274311
name: str,
275312
existing_ids: set[str] | None = None,
276313
) -> str:
277-
tenant_slug = re.sub(r"[^a-z0-9]+", "-", tenant.lower()).strip("-") or "tenant"
278-
name_slug = re.sub(r"[^a-z0-9]+", "-", name.lower()).strip("-") or "agent"
314+
tenant_slug = key_id_slug(tenant, "tenant")
315+
name_slug = key_id_slug(name, "agent")
279316
seen_ids = set(existing_ids or ())
280317
while True:
281318
candidate = f"{tenant_slug}-{name_slug}-{secrets.token_hex(4)}"
@@ -288,7 +325,13 @@ def ensure_key_ids(self, config: ApiKeysConfig) -> bool:
288325
for index, item in enumerate(config.keys):
289326
if item.key_id is not None:
290327
continue
291-
key_id = self.generate_key_id(item.tenant, item.name, existing_ids)
328+
# Derived, not drawn: an id that never reaches the file (a read-only
329+
# key store, or AGENTFLOW_API_KEYS) must come out the same on the next
330+
# load and in every replica. Only a legacy hash-only entry, with
331+
# nothing to derive from, still gets a random one.
332+
key_id = derived_key_id(item, existing_ids) or self.generate_key_id(
333+
item.tenant, item.name, existing_ids
334+
)
292335
existing_ids.add(key_id)
293336
config.keys[index] = item.model_copy(update={"key_id": key_id})
294337
changed = True

‎src/agentflow_runtime/serving/api/auth/manager.py‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ def load(self) -> None:
330330
self._key_rotator.schedule_rotation_cleanup(item)
331331
# Carry windows over by bucket name (`_rate_limit_key`), never by the
332332
# plaintext `keys_by_value` index: a still-configured key keeps its
333-
# window across the reload, a removed key's window is dropped. (T-46)
333+
# window across the reload, a removed key's window is dropped.
334334
live_buckets = {self._rate_limit_key(item) for item in config.keys}
335335
self._rate_windows = defaultdict(
336336
list,
@@ -704,7 +704,6 @@ def _legacy_env_keys(self) -> list[TenantKey]:
704704
key, name = pair, "unnamed"
705705
items.append(
706706
TenantKey(
707-
key_id=self._key_rotator.generate_key_id("default", name.strip(), set()),
708707
key=key.strip(),
709708
name=name.strip(),
710709
tenant="default",
@@ -713,7 +712,12 @@ def _legacy_env_keys(self) -> list[TenantKey]:
713712
created_at=datetime.now(UTC).date(),
714713
)
715714
)
716-
return items
715+
# The same derivation as an id-less key-file entry: from the key's
716+
# lookup digest, so the key keeps its id -- and its rate-limit bucket --
717+
# across reloads, restarts and replicas.
718+
config = ApiKeysConfig(keys=items)
719+
self._key_rotator.ensure_key_ids(config)
720+
return config.keys
717721

718722
def _rate_limit_key(self, tenant_key: TenantKey) -> str:
719723
# This string becomes a Redis sorted-set key NAME (``rate_limiter`` calls

0 commit comments

Comments
 (0)