Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@

### Changed
- ์ˆœ์ˆ˜ ์˜์ˆซ์ž ํ† ํฐ์€ ์ •๊ทœ์‹ ํ˜ธ์ถœ์„ ๊ฑด๋„ˆ๋›ฐ๋˜ ๋‹ค๊ตญ์–ดยท๋ฌธ์žฅ๋ถ€ํ˜ธ ํ† ํฐํ™” ๊ฒฐ๊ณผ๋Š” ๊ธฐ์กด ์˜๋ฏธ์™€ ๋™์ผํ•˜๊ฒŒ ์œ ์ง€ํ•ฉ๋‹ˆ๋‹ค. ๊ทผ๊ฑฐ, ํ•œ๊ณ„, APA 7 ์ฐธ๊ณ ๋ฌธํ—Œ์€ [`docs/doctoring/token-fast-path-equivalence.md`](docs/doctoring/token-fast-path-equivalence.md)์— ๊ธฐ๋กํ–ˆ์Šต๋‹ˆ๋‹ค.
- API ํ‚ค๋Š” ์š”์ฒญ๋งˆ๋‹ค ํ™˜๊ฒฝ ๋ณ€์ˆ˜์—์„œ ๋‹ค์‹œ ์ฝ์ง€ ์•Š๊ณ , ์ผํšŒ์„ฑ ํ™˜๊ฒฝ bootstrap ํ›„ ์ œ๊ฑฐ๋˜๋Š” thread-safe runtime registry์—์„œ ๋ช…์‹œ์ ์œผ๋กœ ๊ด€๋ฆฌํ•ฉ๋‹ˆ๋‹ค. ๋ณด์•ˆ ๊ฒฝ๊ณ„์™€ APA 7 ์ฐธ๊ณ ๋ฌธํ—Œ์€ [`docs/doctoring/api-key-comparison-boundary.md`](docs/doctoring/api-key-comparison-boundary.md)์— ๊ธฐ๋กํ–ˆ์Šต๋‹ˆ๋‹ค.

### Fixed
- ๋‹จ์ผยท์ผ๊ด„ ๋Œ€์ƒ ํฌ๊ธฐ ์ž…๋ ฅ์„ ๋น„์› ์„ ๋•Œ ์ด์ „ custom validity์™€ `aria-invalid` ์ƒํƒœ๋ฅผ ์ฆ‰์‹œ ์ดˆ๊ธฐํ™”ํ•ด ํ˜„์žฌ ํ•„์ˆ˜ ์ž…๋ ฅ ์ƒํƒœ๋ฅผ ์ •ํ™•ํžˆ ์ „๋‹ฌํ•ฉ๋‹ˆ๋‹ค.
- ์—…๋กœ๋“œ ํŒŒ์ผ๋ช…์˜ ๊ฒฝ๋กœ ๊ตฌ๋ถ„์ž๋ฅผ ์ •๊ทœํ™”ํ•˜์—ฌ POSIX์—์„œ๋„ Windows ํ˜•์‹์˜ ํด๋ผ์ด์–ธํŠธ ๊ฒฝ๋กœ๊ฐ€ ์ผ๊ด€๋œ basename์œผ๋กœ ๊ธฐ๋ก๋˜๋„๋ก ์ˆ˜์ •ํ–ˆ์Šต๋‹ˆ๋‹ค.
- API-key middleware๊ฐ€ ๋น„-ASCII ํ—ค๋”๋ฅผ UTF-8 bytes๋กœ ๋น„๊ตํ•˜์—ฌ `hmac.compare_digest()`์˜ ๋ฌธ์ž์—ด `TypeError`๋ฅผ ๋ฐฉ์ง€ํ•˜๊ณ , ์ž˜๋ชป๋œ ํ‚ค๋ฅผ ๋‚ด๋ถ€ ์˜ˆ์™ธ ๋Œ€์‹  ์ผ๊ด€๋œ `401`๋กœ ์ฒ˜๋ฆฌํ•ฉ๋‹ˆ๋‹ค.
86 changes: 86 additions & 0 deletions docs/doctoring/api-key-comparison-boundary.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
# API-key comparison and bootstrap boundary

## Status

Accepted for the API-key middleware change carried by PR #376.

## Problem

`hmac.compare_digest()` accepts either two bytes-like values or two ASCII-only
strings. An HTTP client can supply a non-ASCII header value, so comparing the
raw header string to a configured API-key string can raise `TypeError` before
the middleware emits its ordinary authentication response. An attacker can
repeat that exceptional input to turn an invalid credential into an avoidable
availability failure rather than a bounded `401` response.

The previous request-time environment lookup also made the process environment
a mutable credential source. That mixed one-time bootstrap transport with the
runtime authentication authority and allowed an ambient environment change to
rotate credentials without an explicit application operation.

## Decision

1. API keys are held in a process-local, thread-safe credential registry.
2. `CODEC_CARVER_API_KEYS` is accepted only as one-time bootstrap transport.
3. The bootstrap value is removed from the environment after parsing.
4. Request handling reads only the registry and never rereads the environment.
5. Both the presented key and registered candidates are encoded as UTF-8 bytes
before `hmac.compare_digest()`.
6. Invalid, missing, or non-ASCII presented keys return the same generic `401`
payload and never echo credential material.
7. Empty registries preserve the existing opt-in, unauthenticated development
mode. Production deployments must explicitly provision one or more keys.

This is a fail-closed exceptional-input repair consistent with CWE-754 and with
OWASP ASVS 5.0.0 authentication, secure-coding, and error-handling objectives.
It does not claim that API keys are a complete enterprise identity system.

## Runtime contract

```text
one-time trusted bootstrap
-> parse and normalize keys
-> remove transport environment variable
-> atomically replace registry snapshot
-> middleware reads immutable snapshot
-> UTF-8 byte comparison
-> success or generic 401
```

The registry never writes plaintext keys to disk, logs, response payloads, or
request traces. Key replacement requires an explicit trusted call to
`API_KEY_REGISTRY.set_keys()`.

## Verification contract

The regression suite must prove all of the following:

- an empty registry preserves the existing open-development behavior;
- missing and incorrect keys return `401` without secret disclosure;
- every registered key can authenticate;
- whitespace and empty bootstrap entries are normalized deterministically;
- the environment variable is consumed and removed during bootstrap;
- later environment changes cannot rotate runtime credentials;
- explicit registry replacement changes the accepted key set;
- a non-ASCII header returns `401` instead of raising `TypeError`;
- production statement and branch coverage remain at 100%;
- public production symbols retain complete docstrings.

## Distributed-deployment limit

The in-process registry is intentionally the smallest safe boundary for the
current single-process product. Multi-instance deployment requires a separately
reviewed credential-distribution adapter with versioned updates, instance
convergence evidence, rollback, and secret-manager authorization. Instances
must not poll ambient environment variables on every request. That distributed
rotation design is outside this bounded repair.

## References

Krawczyk, H., Bellare, M., & Canetti, R. (1997). *HMAC: Keyed-hashing for message authentication* (RFC 2104). Internet Engineering Task Force. https://doi.org/10.17487/RFC2104

OWASP Foundation. (2025). *OWASP Application Security Verification Standard 5.0.0*. https://owasp.org/www-project-application-security-verification-standard/

Python Software Foundation. (2026). *hmacโ€”Keyed-hashing for message authentication*. Python 3.14.6 documentation. https://docs.python.org/3/library/hmac.html

The MITRE Corporation. (2026). *CWE-754: Improper check for unusual or exceptional conditions* (Version 4.20). https://cwe.mitre.org/data/definitions/754.html
74 changes: 57 additions & 17 deletions saas_web.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import os
import shutil
import tempfile
import threading
import uuid
import zipfile
from datetime import datetime, timezone
Expand Down Expand Up @@ -83,38 +84,77 @@ async def limited_receive():
except RequestTooLarge:
return JSONResponse(status_code=413, content={"error": "Payload Too Large"})

def get_configured_api_keys():
"""Return the API keys configured via the CODEC_CARVER_API_KEYS env var.
class APIKeyRegistry:
"""Keep runtime API keys in process memory behind a thread-safe boundary.

The variable holds a comma-separated list of keys. Whitespace around each
key is stripped and empty entries are ignored. Keys are read from the
environment at request time (not import time) so tests can patch the
environment easily and key rotation needs no server restart. Returns an
empty list when the variable is unset or contains no usable keys, which
leaves the service open (today's default behaviour).
The environment variable is bootstrap transport only. Creating a registry
consumes and removes that variable so request handling and later environment
changes cannot silently rotate credentials.
"""

raw = os.environ.get("CODEC_CARVER_API_KEYS", "")
return [key.strip() for key in raw.split(",") if key.strip()]
def __init__(self) -> None:
"""Create a registry and consume any one-time environment bootstrap."""

self._keys: tuple[str, ...] = ()
self._lock = threading.Lock()
self._bootstrap_from_environment()

def _bootstrap_from_environment(self) -> None:
"""Load comma-separated keys once, then remove the transport variable."""

raw_keys = os.environ.pop("CODEC_CARVER_API_KEYS", None)
if raw_keys is not None:
self.set_keys(raw_keys.split(","))

def set_keys(self, keys: list[str]) -> None:
"""Atomically replace keys after trimming whitespace and empty entries.

Args:
keys: Candidate plaintext keys supplied by trusted bootstrap or
credential-management code.
"""

normalized_keys = tuple(
key.strip() for key in keys if key.strip()
)
with self._lock:
self._keys = normalized_keys

def get_keys(self) -> list[str]:
"""Return a snapshot of registered keys without exposing mutable state."""

with self._lock:
return list(self._keys)


API_KEY_REGISTRY = APIKeyRegistry()


def get_configured_api_keys() -> list[str]:
"""Return the current in-memory API-key snapshot used by middleware."""

return API_KEY_REGISTRY.get_keys()


@app.middleware("http")
async def require_api_key(request: Request, call_next):
"""Enforce opt-in API-key authentication on all endpoints except GET /.

When one or more keys are configured via CODEC_CARVER_API_KEYS, every
request other than GET / (the upload UI page) must carry an X-API-Key
header matching a configured key; comparison uses hmac.compare_digest to
stay constant-time. Requests failing the check receive a 401 JSON error
without echoing any key material. When no keys are configured, all
requests pass through unchanged.
When one or more keys are present in the in-memory credential registry,
every request other than GET / (the upload UI page) must carry an X-API-Key
header matching a configured key. Byte-oriented hmac.compare_digest keeps
comparison constant-time and prevents non-ASCII header values from raising
TypeError. Failed requests receive a 401 response without echoing key
material; an empty registry preserves the open default.
"""

configured_keys = get_configured_api_keys()
if configured_keys and not (request.method == "GET" and request.url.path == "/"):
provided_key = request.headers.get("x-api-key", "")
provided_key_bytes = provided_key.encode("utf-8")
if not any(
hmac.compare_digest(provided_key, key) for key in configured_keys
hmac.compare_digest(provided_key_bytes, key.encode("utf-8"))
for key in configured_keys
):
return JSONResponse(
status_code=401,
Expand Down
Loading
Loading