apingali Claude Opus 4.7 (1M context) commited on
Commit
37d5cf4
Β·
1 Parent(s): 3c77cd5

fix(security): pass user Anthropic key directly to SDK, never via env

Browse files

CRITICAL multi-tenant fix. The previous code wrote the visitor-supplied
Anthropic API key into os.environ["ANTHROPIC_API_KEY"] before calling
_call_anthropic, which let the SDK pick it up via env. Three real
problems with that on a public Space:

1. os.environ is process-global β€” visitor A's key persisted in env
after their request returned, and was visible to visitor B's
concurrent (or subsequent) request from the same Python worker.
2. Visitor B's call would use visitor A's key, billed to A's account.
3. If visitor B then submitted with no key, the SDK would silently
succeed using A's leaked key instead of erroring as expected.

Fix:
app.py
- _call_anthropic now accepts api_key= keyword-only arg
- When supplied, passes directly to Anthropic(api_key=...) constructor
- When None, calls Anthropic() with no kwargs (SDK reads env as
its normal default β€” used for the Space-owner key path)
diagnose()
- Captures user-supplied key into a local variable; does NOT write
to os.environ
- For provider=anthropic WITH user key, bypasses the generic
dispatcher and calls _call_anthropic(..., api_key=user_key)
directly
- For provider=anthropic WITHOUT user key (env-only path), goes
through the dispatcher as before β€” SDK reads env on its own
- All other providers unchanged

Three new tests covering the regression:
test_diagnose_premium_user_key_passed_directly_not_via_env
Asserts user key goes to backend via kwarg, env stays untouched
even when a Space-owner key is set in env
test_diagnose_premium_does_not_mutate_env_with_user_key
Cross-tenant leak guard β€” env is unset before AND after a
user-key Premium call
test_call_anthropic_passes_api_key_to_sdk_constructor
Asserts the api_key kwarg flows to the Anthropic() constructor
rather than being silently ignored
test_call_anthropic_without_api_key_uses_env_via_sdk
Symmetry: when no api_key supplied, SDK is called with no kwargs
(so it uses its normal env-reading default)

Also updated the existing test (renamed
test_diagnose_premium_user_key_overrides_env β†’
test_diagnose_premium_user_key_passed_directly_not_via_env) to assert
the new contract (key via kwarg, env intact) instead of the old
contract (env mutated to user key).

Total: 62 unit tests pass + 1 skipped opt-in integration.

Privacy disclosure on /labs/compounding-test-ai updated to be honest:
- "The Space code does not write your input or your API key to disk
and does not retain them across requests" (true post-fix)
- "As with any hosted service, HuggingFace's platform may log request
bodies as part of its infrastructure; the Space owner can view
these logs" (true, was previously underplayed)
- "Don't paste anything you wouldn't paste into any third-party LLM
service" with explicit pointer to the deterministic version for
sensitive content

What's NOT a problem (audited but no fix needed):
- File writes β€” none
- localStorage/cookies β€” none, React state is in-memory only
- URL params β€” key never in URL
- Logging β€” we don't log requests; only platform-level logs apply
- Error formatting β€” Anthropic SDK doesn't include keys in errors,
and our F14 wrapper would surface them only to the visitor whose
request caused the error (not other visitors)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Files changed (2) hide show
  1. app.py +29 -6
  2. test_diagnose.py +112 -8
app.py CHANGED
@@ -253,12 +253,19 @@ def _detect_provider(env=None) -> str:
253
  return "anthropic"
254
 
255
 
256
- def _call_anthropic(system_block: str, user_prompt: str) -> str:
257
  """Anthropic backend. System block is cache-marked; the user prompt
258
- is sent fresh. Returns the raw assistant text."""
 
 
 
 
 
 
 
259
  from anthropic import Anthropic
260
 
261
- client = Anthropic()
262
  resp = client.messages.create(
263
  model=ANTHROPIC_MODEL_ID,
264
  max_tokens=2500,
@@ -668,6 +675,7 @@ def diagnose(
668
  # either via the page's API-key field (per-call) or via an
669
  # ANTHROPIC_API_KEY env var on the Space. Without either, fail fast
670
  # with a friendly explanation before we hit the SDK.
 
671
  if provider == "anthropic":
672
  env_key = os.environ.get("ANTHROPIC_API_KEY", "").strip()
673
  user_key = (anthropic_api_key or "").strip()
@@ -679,8 +687,14 @@ def diagnose(
679
  "",
680
  )
681
  if user_key:
682
- # Per-call override; never persisted beyond this request.
683
- os.environ["ANTHROPIC_API_KEY"] = user_key
 
 
 
 
 
 
684
 
685
  user_prompt = (
686
  PROMPT_TEMPLATE
@@ -691,7 +705,16 @@ def diagnose(
691
  )
692
 
693
  try:
694
- raw = _call_model(SYSTEM_BLOCK, user_prompt, provider)
 
 
 
 
 
 
 
 
 
695
  except Exception as e:
696
  # API timeout / rate limit / auth / server / network failure
697
  # (Anthropic SDK, huggingface_hub InferenceClient, or
 
253
  return "anthropic"
254
 
255
 
256
+ def _call_anthropic(system_block: str, user_prompt: str, *, api_key: Optional[str] = None) -> str:
257
  """Anthropic backend. System block is cache-marked; the user prompt
258
+ is sent fresh. Returns the raw assistant text.
259
+
260
+ `api_key`: an optional per-call key. When provided, it goes directly
261
+ to the SDK constructor and is NEVER written to os.environ. This is
262
+ important on a multi-tenant public Space β€” mutating env would leak
263
+ one visitor's key into a concurrent request from another visitor.
264
+ When `api_key` is None, the SDK reads ANTHROPIC_API_KEY from env
265
+ (the Space-owner's key path)."""
266
  from anthropic import Anthropic
267
 
268
+ client = Anthropic(api_key=api_key) if api_key else Anthropic()
269
  resp = client.messages.create(
270
  model=ANTHROPIC_MODEL_ID,
271
  max_tokens=2500,
 
675
  # either via the page's API-key field (per-call) or via an
676
  # ANTHROPIC_API_KEY env var on the Space. Without either, fail fast
677
  # with a friendly explanation before we hit the SDK.
678
+ user_key_for_anthropic: Optional[str] = None
679
  if provider == "anthropic":
680
  env_key = os.environ.get("ANTHROPIC_API_KEY", "").strip()
681
  user_key = (anthropic_api_key or "").strip()
 
687
  "",
688
  )
689
  if user_key:
690
+ # IMPORTANT: do NOT write the user-supplied key to os.environ.
691
+ # That would leak the key into concurrent requests from other
692
+ # visitors on this Space (the process env is shared across
693
+ # all in-flight requests in the Python worker). Instead we
694
+ # pass it directly to _call_anthropic below, which scopes it
695
+ # to a single SDK client instance that's garbage-collected
696
+ # when the call returns.
697
+ user_key_for_anthropic = user_key
698
 
699
  user_prompt = (
700
  PROMPT_TEMPLATE
 
705
  )
706
 
707
  try:
708
+ # When the visitor supplied their own Anthropic key, bypass the
709
+ # generic dispatcher so we can pass the key directly via kwarg
710
+ # without ever touching os.environ. All other paths go through
711
+ # the dispatcher and read credentials from env as usual.
712
+ if provider == "anthropic" and user_key_for_anthropic:
713
+ raw = _call_anthropic(
714
+ SYSTEM_BLOCK, user_prompt, api_key=user_key_for_anthropic,
715
+ )
716
+ else:
717
+ raw = _call_model(SYSTEM_BLOCK, user_prompt, provider)
718
  except Exception as e:
719
  # API timeout / rate limit / auth / server / network failure
720
  # (Anthropic SDK, huggingface_hub InferenceClient, or
test_diagnose.py CHANGED
@@ -370,26 +370,130 @@ def test_diagnose_premium_with_env_key_dispatches_to_anthropic(monkeypatch):
370
  assert parsed["quadrant"] == "compounder"
371
 
372
 
373
- def test_diagnose_premium_user_key_overrides_env(monkeypatch):
374
  """The page's API-key field should take precedence over any
375
- ANTHROPIC_API_KEY env var the Space owner has configured. This is
376
- the F15-ish defense β€” a visitor pasting their own key should never
377
- inadvertently use the Space owner's key."""
 
 
378
  monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-space-owner-xxx")
379
  captured = {}
380
 
381
- def fake_anthropic(system, user):
382
- captured["env_key_at_call_time"] = os.environ.get("ANTHROPIC_API_KEY")
 
383
  return _VALID_BACKEND_RESPONSE
384
 
385
- monkeypatch.setitem(PROVIDERS, "anthropic", fake_anthropic)
386
 
387
  diagnose(
388
  _LONG_DESCRIPTION, None, None, None,
389
  provider="anthropic",
390
  anthropic_api_key="sk-user-yyy",
391
  )
392
- assert captured.get("env_key_at_call_time") == "sk-user-yyy"
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
 
393
 
394
 
395
  def test_diagnose_premium_backend_exception_returns_friendly_error(monkeypatch):
 
370
  assert parsed["quadrant"] == "compounder"
371
 
372
 
373
+ def test_diagnose_premium_user_key_passed_directly_not_via_env(monkeypatch):
374
  """The page's API-key field should take precedence over any
375
+ ANTHROPIC_API_KEY env var the Space owner has configured. Critically,
376
+ the visitor's key must be passed DIRECTLY to _call_anthropic via
377
+ kwarg β€” never written to os.environ β€” or concurrent requests from
378
+ other visitors could pick up the wrong key from shared process env.
379
+ See _call_anthropic docstring."""
380
  monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-space-owner-xxx")
381
  captured = {}
382
 
383
+ def fake_call_anthropic(system, user, *, api_key=None):
384
+ captured["api_key_kwarg"] = api_key
385
+ captured["env_at_call_time"] = os.environ.get("ANTHROPIC_API_KEY")
386
  return _VALID_BACKEND_RESPONSE
387
 
388
+ monkeypatch.setattr(app_module, "_call_anthropic", fake_call_anthropic)
389
 
390
  diagnose(
391
  _LONG_DESCRIPTION, None, None, None,
392
  provider="anthropic",
393
  anthropic_api_key="sk-user-yyy",
394
  )
395
+ # User key passed directly via kwarg (the override mechanism)
396
+ assert captured["api_key_kwarg"] == "sk-user-yyy"
397
+ # CRITICAL: env was NOT clobbered with the user's key β€” Space
398
+ # owner's key remained intact for any concurrent request that
399
+ # legitimately needs it (or for no request at all if there's no
400
+ # owner-set key).
401
+ assert captured["env_at_call_time"] == "sk-space-owner-xxx"
402
+
403
+
404
+ def test_diagnose_premium_does_not_mutate_env_with_user_key(monkeypatch):
405
+ """Cross-tenant key-leak regression test. On a public Space, two
406
+ concurrent visitors may both submit Premium requests. Each must use
407
+ only their own key; neither should ever see the other's key via
408
+ os.environ. The fix is to never write user-supplied keys to env."""
409
+ monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)
410
+ captured = {}
411
+
412
+ def fake_call_anthropic(system, user, *, api_key=None):
413
+ captured["api_key_kwarg"] = api_key
414
+ captured["env_at_call_time"] = os.environ.get("ANTHROPIC_API_KEY")
415
+ return _VALID_BACKEND_RESPONSE
416
+
417
+ monkeypatch.setattr(app_module, "_call_anthropic", fake_call_anthropic)
418
+
419
+ diagnose(
420
+ _LONG_DESCRIPTION, None, None, None,
421
+ provider="anthropic",
422
+ anthropic_api_key="sk-visitor-A-secret",
423
+ )
424
+ # The key went directly to the SDK, not via env
425
+ assert captured["api_key_kwarg"] == "sk-visitor-A-secret"
426
+ # Env was never set during the call
427
+ assert captured["env_at_call_time"] is None
428
+ # And env is still unset after the call returns β€” no residue for
429
+ # the next visitor's concurrent request to pick up
430
+ assert os.environ.get("ANTHROPIC_API_KEY") is None
431
+
432
+
433
+ def test_call_anthropic_passes_api_key_to_sdk_constructor(monkeypatch):
434
+ """When _call_anthropic receives api_key=, it must be passed to the
435
+ Anthropic() SDK constructor β€” not stored in os.environ, not
436
+ discarded, not exposed elsewhere."""
437
+ captured_init = {}
438
+
439
+ class FakeContentBlock:
440
+ text = "ok"
441
+
442
+ class FakeMessage:
443
+ content = [FakeContentBlock()]
444
+
445
+ class FakeClient:
446
+ class messages: # noqa: N801
447
+ @staticmethod
448
+ def create(**kwargs):
449
+ return FakeMessage()
450
+
451
+ def fake_anthropic_ctor(**kwargs):
452
+ captured_init.update(kwargs)
453
+ return FakeClient()
454
+
455
+ import anthropic as anthropic_module
456
+ monkeypatch.setattr(anthropic_module, "Anthropic", fake_anthropic_ctor)
457
+ monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False)
458
+
459
+ _call_anthropic("sys", "usr", api_key="sk-direct-yyy")
460
+
461
+ assert captured_init.get("api_key") == "sk-direct-yyy"
462
+ # And env was not touched
463
+ assert os.environ.get("ANTHROPIC_API_KEY") is None
464
+
465
+
466
+ def test_call_anthropic_without_api_key_uses_env_via_sdk(monkeypatch):
467
+ """When api_key is not supplied, the SDK constructor is called with
468
+ no kwargs β€” letting it read ANTHROPIC_API_KEY from env, as is the
469
+ SDK's normal default behavior. We don't explicitly pass api_key=None
470
+ because the SDK treats that differently than 'not supplied'."""
471
+ captured_init = {}
472
+
473
+ class FakeContentBlock:
474
+ text = "ok"
475
+
476
+ class FakeMessage:
477
+ content = [FakeContentBlock()]
478
+
479
+ class FakeClient:
480
+ class messages: # noqa: N801
481
+ @staticmethod
482
+ def create(**kwargs):
483
+ return FakeMessage()
484
+
485
+ def fake_anthropic_ctor(**kwargs):
486
+ captured_init.update(kwargs)
487
+ return FakeClient()
488
+
489
+ import anthropic as anthropic_module
490
+ monkeypatch.setattr(anthropic_module, "Anthropic", fake_anthropic_ctor)
491
+ monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env-default")
492
+
493
+ _call_anthropic("sys", "usr") # no api_key kwarg
494
+
495
+ # SDK constructor called with no api_key β€” it'll use env on its own
496
+ assert "api_key" not in captured_init
497
 
498
 
499
  def test_diagnose_premium_backend_exception_returns_friendly_error(monkeypatch):