ConWan30 · ConWan30 · Jun 27, 2026
diff --git a/audits/security-review-2026-06-27.md b/audits/security-review-2026-06-27.md
@@ -0,0 +1,152 @@
+# QorTroller Repository Security Review — 2026-06-27
+
+Branch: `claude/repo-vulnerability-analysis-wvwmow`
+
+Scope: full-repo vulnerability analysis across the Python bridge HTTP service, the
+Solidity contract set, the React frontend, committed secrets, and dangerous sinks
+(injection, traversal, SSRF, deserialization). Findings are split into **FIXED in
+this branch** (safe, code-level changes with no on-chain / redeploy impact) and
+**REPORTED for operator action** (deployed/FROZEN contracts and governance items
+that require review + redeployment, which are out of the safe-change envelope for an
+automated audit branch).
+
+---
+
+## 1. FIXED in this branch
+
+### 1.1 CRITICAL — Hardcoded live API key in a public repo (leaked credential)
+The QuickSilver LLM API key `sk-el_TumeRtoQdi-lY-YQmTQ` was hardcoded as a fallback in
+two committed files of a **public** repository:
+
+- `bridge/vapi_bridge/vapi_llm_client.py:19` — used directly as a `Bearer` token.
+- `frontend/src/views/LlmChatView.jsx:94` — baked into the browser bundle and sent
+  on a direct `fetch()` to `api.quicksilverpro.io` from every client.
+
+**Fix applied:** removed both hardcoded fallbacks; the code now fails closed when the
+env var (`QUICKSILVER_API_KEY` / `VITE_QUICKSILVER_API_KEY`) is unset. The key is no
+longer present anywhere in the working tree (`git grep` clean).
+
+**OPERATOR ACTION STILL REQUIRED (cannot be done from code):**
+1. **Rotate/revoke the key at QuickSilver now** — it has been public and must be
+   treated as compromised regardless of removal.
+2. The key remains in **git history** (introduced in commit `7991e11`). Purging it
+   requires a history rewrite + force-push to a public repo, which is on the project's
+   "NEVER without explicit operator request" list — left for an explicit operator
+   decision. Rotation (step 1) is the load-bearing mitigation; history purge is
+   secondary cleanup.
+3. Architectural follow-up: the frontend should never hold a provider key. Proxy LLM
+   calls through the bridge so the secret stays server-side.
+
+### 1.2 HIGH — Empty-key auth bypass + timing oracle on a mutating endpoint
+`bridge/vapi_bridge/operator_api/agent_operator_initiative.py` —
+`POST /operator/evaluate-agent-action` used `if api_key != cfg.operator_api_key`. With
+the default empty `OPERATOR_API_KEY`, an empty `api_key` satisfied `"" != ""` → False →
+auth passed, on an endpoint that runs Cedar evaluation and writes a `shadow_log` row.
+The plain `!=` was also a per-byte timing oracle.
+
+**Fix applied:** fail closed with 503 when no key is configured, and compare with
+`hmac.compare_digest`, matching the `_check_key` pattern used elsewhere.
+
+### 1.3 MEDIUM — Timing-unsafe key comparison on main-app operator endpoints
+`bridge/vapi_bridge/transports/http.py` — five sites (`PATCH /config`,
+`POST /operator/passport`, `POST /operator/passport/issue`, and two others) used plain
+`!=` for key comparison. They already fail closed on the unconfigured case (no bypass),
+but the comparison leaked timing.
+
+**Fix applied:** all five now use `hmac.compare_digest` (added `import hmac`).
+
+### 1.4 MEDIUM — Path-traversal via `startswith` prefix bug (auth-gated)
+`bridge/vapi_bridge/operator_api/agent_misc.py` — the `read_file` tool of
+`POST /agent/local-host/execute` confined paths with
+`safe_path.startswith(normpath(repo_root))`. A bare prefix test admits sibling
+directories whose name extends the repo root (e.g. `../QorTroller-secret/...` resolves
+outside the repo but still starts with `/home/user/QorTroller`).
+
+**Fix applied:** containment now uses `os.path.commonpath([safe_path, repo_root])`.
+Verified against in-repo paths (allowed), `../../../passwd` and `/etc/passwd` (blocked,
+existing test still passes), and the sibling-prefix case (now blocked).
+
+**Verification:** edited modules syntax-check clean; `test_local_host_tools_endpoint.py`
+passes 5/6 (the 1 failure — `read_file` of `cli_chat.py` — is a pre-existing
+checkout/fixture issue that fails identically on the original code, unrelated to these
+changes); `test_operator_initiative_o3_expedite.py` and `_post_o3.py` pass.
+
+---
+
+## 2. REPORTED for operator action (not edited here)
+
+### 2.1 Bridge — read-key fails open when `OPERATOR_API_KEY` is unset (MEDIUM)
+`operator_api/_app.py` `_check_read_key` enforces auth only when a key is configured;
+the default is empty, so a forgotten env var silently exposes all read-only `/agent/*`
+and `/bridge/*` status endpoints. This is documented as intentional dev behavior, so it
+is left as-is. **Recommendation:** add a startup assertion/warning that
+`operator_api_key` is non-empty when HTTP is enabled, or gate unauthenticated reads
+behind an explicit `ALLOW_UNAUTH_READS=1` opt-in.
+
+### 2.2 Bridge — unvalidated `output_dir` write on `POST /operator/vpm-compile` (LOW)
+`operator_api/agent_zkba_vpm.py` passes a body-supplied `output_dir` to the compiler
+unconstrained. Requires the full operator write key (properly `compare_digest`-checked),
+so it is a privileged-operator-only primitive. **Recommendation:** constrain `output_dir`
+to a fixed artifacts root.
+
+### 2.3 Solidity contracts (deployed/FROZEN — require review + redeploy)
+These are real findings but the contracts are deployed on-chain and pinned by PV-CI
+invariants; fixing them safely needs operator review, redeployment, and (where relevant)
+a trusted-setup ceremony. **Do not patch the live bytecode from an audit branch.**
+
+- **H-1 `VAPIQuickSilverCollateral.claimExcessYield()`** (`:147-164`) — computes
+  "excess" as `balanceOf(this) − caller.lockedAmount`, i.e. the whole-contract balance
+  minus only the caller's lock. In a multi-operator deployment one operator can withdraw
+  collateral backing other operators. **Fix:** track `totalLocked`; only
+  `balanceOf(this) − totalLocked` is claimable, distributed pro-rata.
+- **H-2 `BountyMarket.aggregateSwarmReport()`** (`:691-765`) — no access modifier; loops
+  `deviceRegistry.updateReputation(...)` (BountyMarket is an authorized updater) and emits
+  a `PhysicalOracleReport` consensus event. Anyone can inflate any device's corroboration
+  reputation and emit an attacker-chosen "verified" oracle feed. **Fix:** gate the
+  function and require each record be unused evidence already bound to the bounty; de-dup
+  record hashes.
+- **H-3 `VAPIGovernanceTimelock`** (`:201-216`) — `setCoSigner` / `transferOperator` are
+  `onlyOperator` and **not** timelocked, defeating the contract's stated purpose: a stolen
+  operator key can replace the co-signer instantly, then push a malicious operator
+  transition the real co-signer can no longer veto. **Fix:** subject both to the same 48h
+  queue, or require the co-signer's signature to change the co-signer.
+- **H-4 `PoACVerifier.verifyPoAC`** (`:200-434`) — the signed digest is
+  `sha256(body)` with no `chainid` / `address(this)` / caller binding, and verification is
+  permissionless, so a captured `(deviceId, body, sig)` replays across forks/redeploys and
+  contexts. **Fix (respecting the FROZEN 228-byte wire format):** bind context at the
+  on-chain digest — `sha256(body || chainid || address(this))` with matching firmware —
+  and/or require `msg.sender` == device owner. EIP-712-style domain separation.
+- **M-1 `TieredDeviceRegistry._validateAttestationV2`** (`:324`) — manufacturer signature
+  over `keccak256(_pubkey)` with no domain separation; once `attestationEnforced=true`, a
+  single observed signed pair is replayable to claim Attested tier. **Fix:** sign over
+  `keccak256(abi.encode(block.chainid, address(this), _manufacturer, _pubkey))`.
+- **M-2 `ZKSepProofVerifier.verifyAndCheckSnapshot`** (`:111-142`) — proof not bound to
+  `msg.sender`/claimer; replayable if any downstream consumer treats a `true` return as
+  authorization for `claimedPlayerId`. **Fix:** bind claimer into circuit public inputs +
+  per-proof used-marker; range-check the 128-bit hash halves.
+- **Low/Info:** unchecked ERC20 return values in `VAPIOperatorRegistry.slash()` and
+  `VAPIHardwareCertRegistry.certifyHardware` (use `SafeERC20`); single-step operator
+  transfers in several registries (use 2-step); verify upgradeable NFT proxies
+  (`VAPIGamerControllerNFT`, `VAPIOperatorAgentNFT`) are not left uninitialized.
+
+---
+
+## 3. Confirmed clean
+
+- **SQL injection** — the `store/` layer is parameterized throughout; the handful of
+  f-string SQL fragments use a static allowlist or `?,?` placeholders generated from list
+  length with bound tuples. No injection.
+- **Command injection** — no `os.system`; the only HTTP-reachable `subprocess` calls use
+  argument lists (no shell) with hardcoded commands. The daemon's `execute_shell` LLM tool
+  is defended by a metacharacter ban + prefix whitelist + sealed env (`get_sealed_env`) and
+  is reachable only via the agent loop, not anonymous HTTP.
+- **Insecure deserialization** — no `pickle`, `marshal`, unsafe `yaml.load`, `eval`, or
+  `exec` on request/external data anywhere.
+- **SSRF** — no HTTP endpoint fetches a user-controlled URL host.
+- **CORS** — the public forensic sub-app uses `allow_origins=["*"]` with
+  `allow_credentials=False` (correct for read-only public data); the main app restricts
+  origins to localhost + a configurable `FRONTEND_ORIGIN`.
+- **Other committed-secret candidates** — `.env.example` files are placeholders; the
+  `vsd-vault` attestation JSON holds only a public key + public wallet address; AWS keys in
+  tests are obvious fixtures (`AKIATESTTESTTESTTEST`); `0x…` wallet addresses are public.
+  No PEM private keys, mnemonics, Pinata JWTs, or Anthropic keys committed.
@@ -155,8 +155,13 @@ def agent_local_host_execute(
                 path = args.get("path", "")
                 if not path:
                     return {"result": "Error: path parameter is required"}
-                safe_path = os.path.normpath(os.path.join(repo_root_str, path))
-                if not safe_path.startswith(os.path.normpath(repo_root_str)):
+                repo_root_norm = os.path.normpath(repo_root_str)
+                safe_path = os.path.normpath(os.path.join(repo_root_norm, path))
+                # Containment check must use commonpath, NOT startswith: a bare
+                # prefix test admits sibling dirs whose name extends the root
+                # (e.g. "../QorTroller-secret/..." resolves outside the repo but
+                # still startswith("/home/user/QorTroller")).
+                if os.path.commonpath([safe_path, repo_root_norm]) != repo_root_norm:
                     return {"result": "Error: Access denied (path traversal outside workspace)"}
                 if not os.path.exists(safe_path) or os.path.isdir(safe_path):
                     return {"result": "Error: File not found or is a directory"}

@@ -5,6 +5,7 @@
 from __future__ import annotations
 
 import asyncio
+import hmac
 import logging
 import time
 from pathlib import Path
@@ -577,7 +578,12 @@ async def evaluate_agent_action_endpoint(
             policy bundle without waiting for real agent activity
           - Generate baseline shadow log entries for FSCA rule validation
         """
-        if api_key != cfg.operator_api_key:
+        # Fail closed when no key is configured (empty default would otherwise
+        # let an empty api_key pass `"" != ""` → False). Use a constant-time
+        # compare to avoid a timing oracle on the operator key.
+        if not cfg.operator_api_key:
+            raise HTTPException(status_code=503, detail="Operator API not configured")
+        if not hmac.compare_digest(api_key, cfg.operator_api_key):
             raise HTTPException(status_code=403, detail="invalid api_key")
         if len(reason.strip()) < 10:
             raise HTTPException(

@@ -15,6 +15,7 @@
 
 import asyncio
 import hashlib as _hashlib
+import hmac
 import json
 import logging
 import math as _math
@@ -347,7 +348,7 @@ async def patch_config(request: Request):
         x_api_key = request.headers.get("x-api-key", "")
         if not cfg.operator_api_key:
             return JSONResponse({"error": "operator_api_key not configured"}, status_code=503)
-        if x_api_key != cfg.operator_api_key:
+        if not hmac.compare_digest(x_api_key, cfg.operator_api_key):
             return JSONResponse({"error": "unauthorized"}, status_code=401)
         body = await request.json()
         applied, rejected = [], []
@@ -384,7 +385,7 @@ async def operator_passport(request: Request):
         x_api_key = request.headers.get("x-api-key", "")
         if not cfg.operator_api_key:
             return JSONResponse({"error": "operator_api_key not configured"}, status_code=503)
-        if x_api_key != cfg.operator_api_key:
+        if not hmac.compare_digest(x_api_key, cfg.operator_api_key):
             store.log_operator_action("/operator/passport", "", _api_key_hash(x_api_key),
                                       client_ip, 401, "unauthorized")
             return JSONResponse({"error": "unauthorized"}, status_code=401)
@@ -455,7 +456,7 @@ async def issue_passport(request: Request):
         x_api_key = request.headers.get("x-api-key", "")
         if not cfg.operator_api_key:
             return JSONResponse({"error": "operator_api_key not configured"}, status_code=503)
-        if x_api_key != cfg.operator_api_key:
+        if not hmac.compare_digest(x_api_key, cfg.operator_api_key):
             store.log_operator_action("/operator/passport/issue", "", _api_key_hash(x_api_key),
                                       client_ip, 401, "unauthorized")
             return JSONResponse({"error": "unauthorized"}, status_code=401)
@@ -941,7 +942,7 @@ async def agent_override(request: Request):
         x_api_key = request.headers.get("x-api-key", "")
         if not cfg.operator_api_key:
             return JSONResponse({"error": "operator_api_key not configured"}, status_code=503)
-        if x_api_key != cfg.operator_api_key:
+        if not hmac.compare_digest(x_api_key, cfg.operator_api_key):
             return JSONResponse({"error": "unauthorized"}, status_code=401)
         body      = await request.json()
         device_id = body.get("device_id", "")
@@ -991,7 +992,7 @@ async def agent_config(request: Request):
         x_api_key = request.headers.get("x-api-key", "")
         if not cfg.operator_api_key:
             return JSONResponse({"error": "operator_api_key not configured"}, status_code=503)
-        if x_api_key != cfg.operator_api_key:
+        if not hmac.compare_digest(x_api_key, cfg.operator_api_key):
             return JSONResponse({"error": "unauthorized"}, status_code=401)
         body = await request.json()
         if "dry_run" not in body:

@@ -13,10 +13,15 @@ def __init__(self):
         root_env_path = os.path.abspath(os.path.join(current_dir, "..", "..", ".env"))
         load_dotenv(root_env_path)
 
-        # Load API key, fallback to provided key if not set
+        # Load API key from the environment. Never ship a hardcoded credential
+        # fallback — this is a PUBLIC repo, so any baked-in key is world-readable
+        # and must be rotated. Fail closed when the env var is unset.
         self.api_key = os.environ.get("QUICKSILVER_API_KEY")
         if not self.api_key:
-            self.api_key = "sk-el_TumeRtoQdi-lY-YQmTQ"
+            raise RuntimeError(
+                "QUICKSILVER_API_KEY is not set. Configure it in bridge/.env "
+                "(do not commit it). No default credential is provided."
+            )
 
     def _post_completion(self, system_prompt, user_content):
         url = "https://api.quicksilverpro.io/v1/chat/completions"

@@ -91,7 +91,14 @@ export function LlmChatView() {
         const newMessages = [...messages, { role: 'user', content: queryText }]
         setMessages(newMessages)
 
-        const apiKey = import.meta.env.VITE_QUICKSILVER_API_KEY || "sk-el_TumeRtoQdi-lY-YQmTQ"
+        // Never hardcode a credential fallback — this bundle is shipped to every
+        // browser and the repo is public. Require the env var; fail closed.
+        const apiKey = import.meta.env.VITE_QUICKSILVER_API_KEY
+        if (!apiKey) {
+            setErrorMsg('LLM is not configured (VITE_QUICKSILVER_API_KEY is unset).')
+            setLoading(false)
+            return
+        }
 
         // Exclude the initial welcome message and visual tool indicators from LLM prompt
         let payloadMessages = newMessages