T09 · Insecure Skill Coding Practices
- Location
- scripts/main.py:157
- Finding
- NCBI API Key Exposed in Request URLs and Duplicated During Retries<![CDATA[ ## Vulnerability Details **File Location**: `scripts/main.py:157-179` **Vulnerability Type**: Credential exposure through URL query parameters and unsafe recursive retry **Risk Level**: Medium ### Complete Code Snippet ```python def _ncbi_request(self, url: str) -> Optional[Dict]: """Make NCBI API request with rate limiting and error handling.""" self._rate_limit() if self.api_key: url = f"{url}&api_key={self.api_key}" if "?" in url else f"{url}?api_key={self.api_key}" try: req = urllib.request.Request( url, headers={ "User-Agent": "VariantAnnotator/1.0 (academic research)", "Accept": "application/json" } ) with urllib.request.urlopen(req, timeout=30) as response: return json.loads(response.read().decode('utf-8')) except urllib.error.HTTPError as e: if e.code == 429: time.sleep(1) return self._ncbi_request(url) return None except Exception: return None ``` ### Technical Analysis The NCBI API key is inserted into the complete request URL. Query strings can be retained by HTTP access logs, proxies, debugging systems, monitoring platforms, exception diagnostics, or process instrumentation. Although HTTPS protects the request in transit, it does not prevent the endpoint or trusted intermediary infrastructure from recording the URL. The HTTP 429 handler recursively calls `_ncbi_request(url)` with a URL that already contains the API key. The next invocation appends the same key again. Repeated rate-limit responses therefore create URLs containing multiple copies of the credential and cause unbounded recursion. This can eventually result in oversized URLs, excessive delays, or a `RecursionError`. ### Attack Path 1. A user supplies an NCBI key through `--api-key`. 2. The Skill appends the key to each NCBI request URL. 3. A proxy, monitoring system, exception coll ...[truncated 714 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Avoid recording or displaying request URLs containing API credentials. - Construct each retry from an immutable base URL rather than passing an already modified URL back into the function. - Add the API key exactly once using a structured query-parameter builder. - Redact `api_key` values in application, proxy, and diagnostic logs. - Replace recursive retries with a bounded iterative retry loop. - Set a maximum retry count and use exponential backoff with jitter. - Honor the HTTP `Retry-After` header when it is present. - Return a clear, sanitized error after the retry budget is exhausted. ]]>
