Skip to content

Commit d6b3a73

Browse files
rustyconoverclaude
andcommitted
serve: drop the introspection rate limit
vgi-rpc removed the per-caller limiter from vgi_rpc.Identity.v1: the allowlisted caller is a proxy introspecting on behalf of every client, so the per-caller budget was one budget for every user's login, drainable by unauthenticated junk credentials, while bounding only guessing -- which a random credential defeats at any rate. Removes `create_app(introspect_rate_limit=)`, `--introspect-rate-limit`, `VGI_INTROSPECT_RATE_LIMIT` and its validation, and the env republish for granian children. vgi-rpc keeps accepting the keyword for one release as a deprecated no-op because 0.34.1 passes it; this version does not, which a test pins by building the identity under an error filter for DeprecationWarning. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent db720a5 commit d6b3a73

3 files changed

Lines changed: 20 additions & 72 deletions

File tree

‎CLAUDE.md‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -298,7 +298,6 @@ vgi-client --input data.parquet --function sum_all_columns --worker vgi-fixture-
298298
| `VGI_OAUTH_DEVICE_CODE_CLIENT_SECRET` | Client secret for device-code flow (optional, URL-safe chars only) |
299299
| `VGI_OAUTH_USE_ID_TOKEN` | When `1`/`true`/`yes`, clients use OIDC `id_token` as Bearer instead of `access_token` |
300300
| `VGI_INTROSPECT_PRINCIPALS` | Comma-separated principals permitted to call `vgi_rpc.Identity.v1`'s `introspect_token`. Required — no permissive default — whenever the worker implements `resolve_token()` (see below) |
301-
| `VGI_INTROSPECT_RATE_LIMIT` | Introspection requests allowed per caller per second (default 20) |
302301
| `VGI_WORKER_ACCESS_LOG_SAMPLE` | Fraction of *successful* calls to keep in the access log, `0.0`–`1.0`. Errors are always kept; the decision is per call, so every record of one stream shares a fate |
303302
| `VGI_WORKER_ACCESS_LOG_ASYNC` | When `1`/`true`/`yes`, emit access-log records from a listener thread. Bounded queue; full means drop, and a crash loses whatever is queued |
304303
| `VGI_WORKER_ACCESS_LOG_QUEUE_SIZE` | Bound on the async access-log queue (default 10000) |
@@ -461,8 +460,10 @@ Enabling it also requires an allowlist of principals permitted to ask, via
461460
default and a worker that overrides the hook without one **refuses to start**:
462461
authenticating and introspecting are different capabilities, and "any
463462
authenticated caller" lets any user resolve any other user's credential to its
464-
owner. `--introspect-rate-limit` (default 20/caller/second) bounds, rather than
465-
closes, the oracle an allowlisted-but-compromised caller still has.
463+
owner. Introspection is deliberately not rate limited (vgi-rpc removed the
464+
limiter): the allowlisted caller is a proxy asking on behalf of every client, so a
465+
per-caller limit was one budget for every user's login, drainable by junk
466+
credentials. Throttle untrusted traffic at the proxy, per client.
466467

467468
Return `None` for "the store answered and this credential is unknown"; raise
468469
`AuthUnavailableError` for "the answer is not knowable". A caller that

‎tests/test_serve.py‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -551,19 +551,18 @@ class TestTokenIntrospection:
551551
*wiring*: a ``resolve_token`` override becomes an ``IdentityImpl`` carrying
552552
the configured allowlist, and nothing at all when the hook is absent. The
553553
guards themselves (allowlist enforcement, uniform rejection, JWS-shape
554-
refusal, rate limiting) live in vgi-rpc and are tested there.
554+
refusal) live in vgi-rpc and are tested there.
555555
"""
556556

557557
@staticmethod
558558
def _identity(worker_cls: type[Worker], **env: str) -> object | None:
559559
from vgi.serve import _build_identity
560560

561561
with pytest.MonkeyPatch.context() as mp:
562-
for key in ("VGI_INTROSPECT_PRINCIPALS", "VGI_INTROSPECT_RATE_LIMIT"):
563-
mp.delenv(key, raising=False)
562+
mp.delenv("VGI_INTROSPECT_PRINCIPALS", raising=False)
564563
for key, value in env.items():
565564
mp.setenv(key, value)
566-
return _build_identity(worker_cls, None, None)
565+
return _build_identity(worker_cls, None)
567566

568567
def test_identity_absent_without_hook(self) -> None:
569568
"""Not "hosted and refusing" — absent.
@@ -641,14 +640,19 @@ def test_hook_without_allowlist_refuses_to_start(self) -> None:
641640
create_app(_IntrospectingWorker, prefix="/vgi", describe=False)
642641
assert exc.value.code == 1
643642

644-
@pytest.mark.parametrize("bad", ["nonsense", "0", "-5"])
645-
def test_bad_rate_limit_refuses_to_start(self, bad: str) -> None:
646-
"""A typo must not silently become "refuse everything" or "no bound"."""
647-
with pytest.MonkeyPatch.context() as mp:
643+
def test_the_retired_rate_limit_is_not_passed(self) -> None:
644+
"""Introspection is not rate limited, and this repo no longer asks for it.
645+
646+
vgi-rpc still accepts ``introspect_rate_limit=`` for one release as a
647+
deprecated no-op, only because vgi-python 0.34.1 passes it. Building the
648+
identity without a ``DeprecationWarning`` pins that this version does not.
649+
"""
650+
import warnings
651+
652+
with pytest.MonkeyPatch.context() as mp, warnings.catch_warnings():
653+
warnings.simplefilter("error", DeprecationWarning)
648654
mp.setenv("VGI_INTROSPECT_PRINCIPALS", "proxy-a")
649-
mp.setenv("VGI_INTROSPECT_RATE_LIMIT", bad)
650-
with pytest.raises(SystemExit):
651-
create_app(_IntrospectingWorker, prefix="/vgi", describe=False)
655+
create_app(_IntrospectingWorker, prefix="/vgi", describe=False)
652656

653657

654658
class TestAuthUnavailableReExport:

‎vgi/serve.py‎

Lines changed: 1 addition & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,6 @@ def create_app(
217217
max_stream_response_bytes: int | None = None,
218218
max_externalized_response_bytes: int | None = None,
219219
introspect_principals: Iterable[str] | None = None,
220-
introspect_rate_limit: int | None = None,
221220
iroh_bridge_issuer: str | None = None,
222221
iroh_trusted_proxy_addresses: Iterable[str] = (),
223222
iroh_authenticate: bool = True,
@@ -279,9 +278,6 @@ def create_app(
279278
``vgi_rpc.Identity.v1``'s ``introspect_token``. Only consulted
280279
when the worker class overrides ``resolve_token``. ``None`` reads
281280
``VGI_INTROSPECT_PRINCIPALS``.
282-
introspect_rate_limit: Introspection requests allowed per caller per
283-
second. ``None`` reads ``VGI_INTROSPECT_RATE_LIMIT``, defaulting
284-
to 20.
285281
iroh_bridge_issuer: Operator-controlled identity namespace for an
286282
``iroh-http/2`` bridge. ``None`` disables Iroh header trust.
287283
iroh_trusted_proxy_addresses: Exact immediate bridge addresses.
@@ -327,7 +323,7 @@ def create_app(
327323
worker,
328324
enable_describe=describe,
329325
server_version=_get_vgi_version(),
330-
identity=_build_identity(worker_cls, introspect_principals, introspect_rate_limit),
326+
identity=_build_identity(worker_cls, introspect_principals),
331327
)
332328

333329
effective_peer_identity_providers = tuple(peer_identity_providers)
@@ -499,15 +495,6 @@ def serve(
499495
"that case, with no permissive default. Env: VGI_INTROSPECT_PRINCIPALS."
500496
),
501497
),
502-
introspect_rate_limit: int | None = typer.Option( # noqa: B008
503-
None,
504-
"--introspect-rate-limit",
505-
help=(
506-
"Introspection requests allowed per caller per second (default 20). "
507-
"Bounds, rather than closes, the oracle an allowlisted-but-compromised "
508-
"caller has. Env: VGI_INTROSPECT_RATE_LIMIT."
509-
),
510-
),
511498
access_log_sample: float | None = typer.Option( # noqa: B008
512499
None,
513500
"--access-log-sample",
@@ -591,7 +578,6 @@ def serve(
591578
max_stream_response_bytes=max_stream_response_bytes,
592579
max_externalized_response_bytes=max_externalized_response_bytes,
593580
introspect_principals=introspect_principals,
594-
introspect_rate_limit=introspect_rate_limit,
595581
server=server,
596582
worker_ref=worker_ref,
597583
http_workers=http_workers,
@@ -802,7 +788,6 @@ def _resolve_authenticate() -> Callable[..., Any] | None:
802788
def _build_identity(
803789
worker_cls: type[Worker],
804790
introspect_principals: Iterable[str] | None,
805-
introspect_rate_limit: int | None,
806791
) -> IdentityImpl | None:
807792
"""Build the ``vgi_rpc.Identity.v1`` implementation, or ``None``.
808793
@@ -823,8 +808,6 @@ def _build_identity(
823808
override.
824809
introspect_principals: Principals permitted to introspect, or ``None``
825810
to read the environment.
826-
introspect_rate_limit: Per-caller, per-second ceiling, or ``None`` to
827-
read the environment.
828811
829812
Returns:
830813
The implementation, or ``None`` when this worker does not resolve
@@ -842,7 +825,6 @@ def _build_identity(
842825
return IdentityImpl(
843826
resolve_token=resolver,
844827
introspect_principals=_resolve_introspect_principals(introspect_principals),
845-
introspect_rate_limit=_resolve_introspect_rate_limit(introspect_rate_limit),
846828
)
847829

848830

@@ -891,42 +873,6 @@ def _resolve_introspect_principals(explicit: Iterable[str] | None) -> list[str]:
891873
return principals
892874

893875

894-
def _resolve_introspect_rate_limit(explicit: int | None) -> int:
895-
"""Resolve introspection requests allowed per caller per second.
896-
897-
Env var: ``VGI_INTROSPECT_RATE_LIMIT``; defaults to 20.
898-
899-
Args:
900-
explicit: Value passed to :func:`create_app`, or None to read the
901-
environment.
902-
903-
Returns:
904-
A positive per-caller, per-second request ceiling.
905-
906-
Raises:
907-
SystemExit: When the environment value is not a positive integer. A
908-
typo that silently became 0 would refuse every introspection with
909-
no diagnostic; one that became huge would remove the bound on an
910-
allowlisted-but-compromised caller's guessing rate.
911-
912-
"""
913-
if explicit is not None:
914-
value = explicit
915-
else:
916-
raw = os.environ.get("VGI_INTROSPECT_RATE_LIMIT")
917-
if not raw:
918-
return 20
919-
try:
920-
value = int(raw)
921-
except ValueError:
922-
sys.stderr.write(f"Error: VGI_INTROSPECT_RATE_LIMIT must be an integer, got {raw!r}\n")
923-
sys.exit(1)
924-
if value <= 0:
925-
sys.stderr.write(f"Error: introspection rate limit must be positive, got {value}\n")
926-
sys.exit(1)
927-
return value
928-
929-
930876
def _resolve_proxy_proof_gate() -> Any | None:
931877
"""Build a proxy-proof gate from ``VGI_PROXY_PROOF_*`` environment variables.
932878
@@ -1506,7 +1452,6 @@ def _serve_http(
15061452
max_stream_response_bytes: int | None = None,
15071453
max_externalized_response_bytes: int | None = None,
15081454
introspect_principals: str | None = None,
1509-
introspect_rate_limit: int | None = None,
15101455
server: str = "waitress",
15111456
worker_ref: str | None = None,
15121457
http_workers: int | None = None,
@@ -1524,8 +1469,6 @@ def _serve_http(
15241469
# export_serve_config for why the environment is the channel.
15251470
if introspect_principals is not None:
15261471
os.environ["VGI_INTROSPECT_PRINCIPALS"] = introspect_principals
1527-
if introspect_rate_limit is not None:
1528-
os.environ["VGI_INTROSPECT_RATE_LIMIT"] = str(introspect_rate_limit)
15291472

15301473
if server == "granian":
15311474
_serve_http_granian(

0 commit comments

Comments
 (0)