mirror of
https://github.com/superdesigndev/treg.git
synced 2026-10-02 03:24:35 +08:00
fix(ci): stop committing a real key, and make the JWT rule linear on provider data
Three CI findings, three different kinds of problem. gitleaks flagged a Fernet key hardcoded in e2e-server.sh. That one is real: it encrypts only a throwaway local e2e.db and prod reads TREG_SECRET_KEY from the environment, so the blast radius is a local database — but a committed key is a committed key. treg-dev-server hardcodes one because its database persists and a new key would orphan stored secrets; this harness deletes its database between runs, so it now generates a fresh key per run and needs no constant at all. The other two were the same fixture: a Stripe-shaped string in a test whose whole job is to prove such strings get masked. Now reuses the placeholder .gitleaks.toml already allowlists for that exact purpose, so a test about masking secrets stops tripping the secret scanner and the allowlist stops growing a line per test. CodeQL flagged the JWT alternative as polynomial on uncontrolled data, and it was right — the alternative was written for argv and round 2 moved it onto PROVIDER response bodies. Measured on 'eyJ' repeated: 1.1ms at 2KB, 4.2ms at 4KB, 17ms at 8KB — quadratic, attacker-triggerable, on the request path, once per failed call. Anchoring with \b leaves one start position instead of one every three characters, and a possessive quantifier removes backtracking within an attempt (it cannot change what matches: the class excludes '.', so the run always ends at the first one). Unmeasurable at every size after, and real JWTs still mask. Fixed in the argv rule too — same shape, same exposure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
b1231de587
commit
3e902adf78
+8
-1
@@ -21,10 +21,17 @@ esac
|
||||
# Real provider keys, so a real upstream answers with a real error body — the whole point of e2e.
|
||||
set -a; . "$HERE/.env"; set +a
|
||||
|
||||
# A FRESH Fernet key per run, generated here rather than written into the file. `treg-dev-server`
|
||||
# hardcodes one because its database persists and a new key would orphan every stored secret; this
|
||||
# harness deletes its database between runs, so it has no such need — and a committed key is a
|
||||
# committed key, whatever it unlocks. (gitleaks flagged the hardcoded one, correctly.)
|
||||
SECRET_KEY="$(uv run --frozen --directory "$HERE" python -c \
|
||||
'from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())')"
|
||||
|
||||
PORT="$PORT" \
|
||||
TREG_E2E_MARKER=1 \
|
||||
TREG_DATABASE_URL="sqlite+aiosqlite:///$HERE/e2e.db" \
|
||||
TREG_SECRET_KEY="GENERATED-AT-RUNTIME" \
|
||||
TREG_SECRET_KEY="$SECRET_KEY" \
|
||||
TREG_ADMIN_TOKEN="E2E-ADMIN-TOKEN" \
|
||||
nohup uv run --frozen --directory "$HERE" python -m treg > "$LOG" 2>&1 &
|
||||
|
||||
|
||||
+9
-2
@@ -6346,7 +6346,12 @@ async def delete_tool(
|
||||
# that follows a credential-looking flag (so a SHORT password like `--password hunter2` is masked too).
|
||||
_ARGV_SECRET_RE = re.compile(
|
||||
r"\b(?:sk|pk|rk|ghp|gho|ghs|ghu|glpat|AKIA|ASIA|AIza|xox[baprs])[A-Za-z0-9_\-]{6,}\b"
|
||||
r"|eyJ[A-Za-z0-9_\-]+\.[A-Za-z0-9_.\-]{8,}" # JWT (base64url with dots)
|
||||
# JWT (base64url with dots). `\b` and the POSSESSIVE `++` are load-bearing, not tidying: without
|
||||
# the anchor, input like "eyJeyJeyJ…" offers a fresh start position every three characters and
|
||||
# each one scans forward for a `.`, which is quadratic. Anchoring leaves one start; `++` removes
|
||||
# backtracking within an attempt (it cannot change what matches here — the class excludes `.`,
|
||||
# so the run always ends at the first one). Same shape guards the argv rule below.
|
||||
r"|\beyJ[A-Za-z0-9_\-]++\.[A-Za-z0-9_.\-]{8,}"
|
||||
r"|\b[A-Za-z0-9_\-]{24,}\b") # any 24+ high-entropy run — deliberately over-masks (git SHAs, UUIDs)
|
||||
# since in an audit log a false mask is harmless but a real key isn't
|
||||
_CRED_FLAG = r"--?(?:token|password|passwd|pass|pwd|api[-_]?key|secret|auth|bearer|credential)s?"
|
||||
@@ -9292,7 +9297,9 @@ _URL_USERINFO_RE = re.compile(r"://[^/\s:@]+:[^/\s@]+@")
|
||||
# that a third-party secret may occasionally survive here.
|
||||
_EVIDENCE_SECRET_RE = re.compile(
|
||||
r"\b(?:sk|pk|rk|ghp|gho|ghs|ghu|glpat|AKIA|ASIA|AIza|xox[baprs])[A-Za-z0-9_\-]{6,}\b"
|
||||
r"|eyJ[A-Za-z0-9_\-]+\.[A-Za-z0-9_.\-]{8,}")
|
||||
# Anchored + possessive for the same reason as the argv rule above, and it matters MORE here:
|
||||
# this one runs on a PROVIDER's response body, which is uncontrolled input on the request path.
|
||||
r"|\beyJ[A-Za-z0-9_\-]++\.[A-Za-z0-9_.\-]{8,}")
|
||||
|
||||
# Response headers worth keeping on a FAILED platform call. An empty-bodied 401 or 429 is otherwise
|
||||
# undiagnosable, and these say which of "bad credential" / "wrong scheme" / "quota gone" / "retry in
|
||||
|
||||
@@ -294,14 +294,22 @@ async def test_a_provider_correlation_id_survives_redaction(clients: AsyncClient
|
||||
|
||||
|
||||
async def test_a_real_secret_shape_is_still_masked(clients: AsyncClient, platform_on, monkeypatch):
|
||||
"""Relaxing the catch-all must not relax the targeted rules: known prefixes and JWTs still go."""
|
||||
"""Relaxing the catch-all must not relax the targeted rules: known prefixes and JWTs still go.
|
||||
|
||||
The fixture is the placeholder `.gitleaks.toml` already allowlists for exactly this purpose
|
||||
("proves output redaction masks a key") rather than a fresh invented one — a test whose job is
|
||||
to prove secrets get masked should not itself trip the secret scanner, and reusing the existing
|
||||
entry keeps the allowlist from growing one line per test that needs a key-shaped string.
|
||||
"""
|
||||
fake_key = "sk_live_ABCDEFGHIJKLMNOP1234"
|
||||
fake_jwt = "eyJhbGciOi.JIUzI1NiIsInR5cCI6"
|
||||
monkeypatch.setattr(A, "relay", _fake_relay(
|
||||
400, b'{"message":"bad token sk_live_ABCDEFGHIJKLMNOP1234 and eyJhbGciOi.JIUzI1NiIsInR5cCI6"}'))
|
||||
400, f'{{"message":"bad token {fake_key} and {fake_jwt}"}}'.encode()))
|
||||
r = await clients.get(f"/call/{EP}?aweme_id=7")
|
||||
assert r.status_code == 400
|
||||
row = await _row(clients)
|
||||
assert "sk_live_ABCDEFGHIJKLMNOP1234" not in row["error_response"]
|
||||
assert "eyJhbGciOi.JIUzI1NiIsInR5cCI6" not in row["error_response"]
|
||||
assert fake_key not in row["error_response"]
|
||||
assert fake_jwt not in row["error_response"]
|
||||
|
||||
|
||||
async def test_a_bodyless_failure_still_leaves_a_row(clients: AsyncClient, platform_on, monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user