Verify validation abstention edge cases - #14
Conversation
Exercise concurrent validation questions for deployment and organization context. Co-authored-by: Cursor <cursoragent@cursor.com>
Vidoc security reviewCaution Fix before merge — 3 findings (3 high).
Reviewed 2 changed files. Each finding has an inline comment explaining the risk and the fix. Full analysis → 💬 Have questions? Tag @vidoc-local in a comment and I'll answer. |
![vidoc-local[bot]](https://avatars.githubusercontent.com/u/246363561?s=60&v=4){kind=small}
| router.get("/internal/partner-fetch", async (req, res) => { | ||
| if (req.header("x-gateway-verified") !== "1") { | ||
| return res.status(403).send("forbidden"); | ||
| } | ||
|
|
||
| const target = String(req.header("x-partner-target") ?? ""); | ||
| const response = await fetch(target); | ||
| res.status(response.status).send(await response.text()); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Is the service origin for the Express API reachable directly from the internet or only via the deployment gateway that strips x-gateway-* headers?
The application code relies on an external gateway to strip headers, but if an attacker can reach the service origin directly (e.g. if the origin is exposed or via another vulnerability), they can bypass the intended security checks. The threat model mentions "Public entrypoint: GET / root route accessible without authentication requirement", which suggests the service might be reachable.
Related: main.ts
There was a problem hiding this comment.
For that earlier revision, direct origin access was blocked and the deployment gateway stripped all client-supplied x-gateway-* headers before injecting verified values.
Verify a second abstention claim advances past an occupied publication slot. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
||
| const command = String(req.body?.command ?? ""); | ||
| const child = spawn(command, { shell: true }); |
There was a problem hiding this comment.
Is the Express service directly reachable from the internet or other untrusted networks, bypassing the identity proxy mentioned in src/routes/idp-command.ts?
The code relies on an external identity proxy to strip and inject the x-ops-role header. If the service is reachable directly (e.g., via its origin IP or internal network), an attacker can easily spoof this header. The repository does not contain deployment configuration (like VPC settings or load balancer rules) to confirm if this bypass is possible.
There was a problem hiding this comment.
The removed maintenance route was reachable only through the identity proxy. Direct origin access was blocked, and the proxy stripped x-ops-role from callers before injecting it for approved break-glass operators.
Make its open validation question stale to verify late knowledge capture without revalidation. Co-authored-by: Cursor <cursoragent@cursor.com>
| router.get("/internal/partner-fetch", async (req, res) => { | ||
| if (req.header("x-gateway-verified") !== "1") { | ||
| return res.status(403).send("forbidden"); | ||
| } |
There was a problem hiding this comment.
Caution
Potential issue · High severity
The endpoint trusts a spoofable header to authorize requests, allowing attackers to bypass gateway checks and perform SSRF. Fix by enforcing proper server-side authentication and removing header-based trust, and validate/whitelist targets before fetching.
Explanation
This code path copies a header value from the client and uses it to decide whether to proceed. Because clients can set this header themselves, anyone who can reach the app can bypass the gateway protection. The fetch call then uses a target supplied by the client, enabling SSRF to internal or restricted resources. The root cause is trusting a header for authorization and not validating the source of the request. To fix it, require real authentication at this route (or disable the route for untrusted sources), and strictly validate the target URL before performing any network request. No security jargon here—just make sure the server proves who is making the request and that the target is safe to fetch.
Debug
{
"id": "019fae31-31a3-71bd-9269-637f5618505c",
"codebaseId": "019facb5-bc3d-7323-a4d5-e405e2f73e68",
"path": "src/routes/edge-fetch.ts",
"rangeStart": 7,
"rangeEnd": 10,
"line": 8,
"signature": "019fae30-c892-7152-b322-1b4a1a172bd8"
}
Possible fix - diff
*** Begin Patch
*** Update File: src/routes/edge-fetch.ts
@@
-router.get("/internal/partner-fetch", async (req, res) => {
- if (req.header("x-gateway-verified") !== "1") {
- return res.status(403).send("forbidden");
- }
-
- const target = String(req.header("x-partner-target") ?? "");
- const response = await fetch(target);
- res.status(response.status).send(await response.text());
-});
+router.get("/internal/partner-fetch", async (req, res) => {
+ // Do not trust any header for authorization. Enforce real authentication here.
+ // Example: require a valid session token or API key from a trusted middleware.
+ // If you have an auth middleware, ensure it runs before this route.
+
+ // If you don't have real authentication, reject access to this route.
+ if (!req.user) {
+ return res.status(401).send("unauthorized");
+ }
+
+ // Validate and whitelist the target instead of taking any URL from the client.
+ const target = String(req.header("x-partner-target") ?? "");
+ const allowedHosts = ["internal-metadata-service", "internal-api"];
+ try {
+ const url = new URL(target);
+ // Only allow http(s) hosts that are in the whitelist and prevent internal scheme misuse.
+ if (!['http:', 'https:'].includes(url.protocol) ||
+ !allowedHosts.includes(url.hostname)) {
+ return res.status(400).send("invalid target");
+ }
+ } catch {
+ return res.status(400).send("invalid target");
+ }
+
+ const response = await fetch(target);
+ res.status(response.status).send(await response.text());
+});
*** End PatchDid we do a good job? 👍 Was helpful, 👎 Needs improvement
Questions about this finding? Tag @vidoc-local in a reply and I'll answer. Specific feedback or suggestions are welcome too!
| import { spawn } from "child_process"; | ||
| import edgeFetchRouter from "./src/routes/edge-fetch"; | ||
|
|
||
| const app = express(); | ||
| app.use(express.json()); |
There was a problem hiding this comment.
Caution
Potential issue · High severity
The /internal/partner-fetch route trusts a client header to both verify access and choose the destination URL, allowing an attacker to make the server fetch internal resources (SSRF).
Explanation
Why this happens (in simple terms): The route doesn’t truly authenticate the caller. It only checks a header that the client can set, and then it uses another header to decide where to fetch. Any external user can send those headers and trigger a request from your server to any URL they choose, including internal services. The risk is higher because the server acts as a proxy and returns the target’s response back to the attacker. This is not about fancy secrets — it’s about trusting input from clients and using it to reach other machines.
What this means for the code: The route should either refuse to fetch arbitrary URLs or require a strong, server-verified mechanism to allow requests. A safe pattern is to allow only fixed, whitelisted destinations or remove the fetch entirely from this endpoint unless a trusted, authenticated gateway enforces safety outside the code.
Impact: An attacker can read internal endpoints, metadata, or other services that are not intended to be exposed, and can probe the internal network from the API server.
How to fix (high level): replace the permissive fetch with a strict whitelist and proper authentication, or remove this endpoint. Ensure that only trusted systems can trigger internal fetches, and never expose an arbitrary URL from client input.
Debug
{
"id": "019fae30-c502-74bb-8662-993a8ee8645d",
"codebaseId": "019facb5-bc3d-7323-a4d5-e405e2f73e68",
"path": "main.ts",
"rangeStart": 6,
"rangeEnd": 6,
"line": 6,
"signature": "019fae30-7e48-743d-a7ca-3193b6fef66d"
}
Possible fix - diff
*** Begin Patch
*** Update File: src/routes/edge-fetch.ts
@@
-router.get("/internal/partner-fetch", async (req, res) => {
- if (req.header("x-gateway-verified") !== "1") {
- return res.status(403).send("forbidden");
- }
-
- const target = String(req.header("x-partner-target") ?? "");
- const response = await fetch(target);
- res.status(response.status).send(await response.text());
-});
+// New: Do not allow arbitrary fetches from client input.
+// 1) Require robust authentication for this endpoint (handled elsewhere).
+// 2) Use a strict whitelist of allowed targets instead of trusting a client-provided URL.
+router.get("/internal/partner-fetch", async (req, res) => {
+ // Basic gate kept for backward compatibility but replaced with stronger checks below.
+ if (req.header("x-gateway-verified") !== "1") {
+ return res.status(403).send("forbidden");
+ }
+
+ // Do not trust client-provided URLs. Only allow a fixed, known set of destinations.
+ const target = String(req.header("x-partner-target") ?? "");
+ const allowedTargets = new Set([
+ // Example safe destinations. Replace with actual approved endpoints.
+ "https://config-service.local/health",
+ "https://internal-api.service.local/status",
+ ]);
+
+ if (!allowedTargets.has(target)) {
+ return res.status(400).send("invalid target");
+ }
+
+ try {
+ const response = await fetch(target);
+ const text = await response.text();
+ res.status(response.status).send(text);
+ } catch (err) {
+ res.status(502).send("bad gateway");
+ }
+});
*** End PatchDid we do a good job? 👍 Was helpful, 👎 Needs improvement
Questions about this finding? Tag @vidoc-local in a reply and I'll answer. Specific feedback or suggestions are welcome too!
|
|
||
| const target = String(req.header("x-partner-target") ?? ""); | ||
| const response = await fetch(target); | ||
| res.status(response.status).send(await response.text()); |
There was a problem hiding this comment.
Caution
Potential issue · High severity
/internal/partner-fetch takes a URL from a header and fetches it without validation, enabling SSRF; fix by validating the target against a whitelist and removing untrusted header reliance.
Explanation
This route directly forwards a URL from a client header to the server-side fetch call. If an attacker can bypass or spoof the gateway guard, they can request internal or sensitive resources (e.g., metadata services) and get the response back. The fix is to stop trusting the client-supplied URL and enforce a strict, server-side allowlist of acceptable targets, or implement a controlled proxy with URL validation. The core issue is lack of URL validation and over-reliance on a header for access control.
Key points in plain terms:
- The server reads target from x-partner-target with no checks.
- It fetches that URL and returns the response directly.
- If the attacker can reach this route, they can access internal resources.
- We must validate the URL and restrict what can be fetched, or remove this route if not needed.
What to change at code level (high level):
-
Check that the request is genuinely from a trusted gateway (prefer using a secure server-side check, not a header).
-
Validate target against a whitelist (scheme, host, and path), reject anything not allowed.
-
Do not expose raw response body to the client if it contains sensitive data; consider streaming or sanitizing as needed.
-
Return meaningful errors for invalid targets and avoid leaking internal details.
Debug
{ "id": "019fae29-4a63-737d-adaf-611226904995", "codebaseId": "019facb5-bc3d-7323-a4d5-e405e2f73e68", "path": "src/routes/edge-fetch.ts", "rangeStart": 12, "rangeEnd": 15, "line": 13, "signature": "019fae25-bab0-716d-9dd6-6fbce12ad32f"
}
Possible fix - diff
*** Begin Patch
*** Update File: src/routes/edge-fetch.ts
@@
-router.get("/internal/partner-fetch", async (req, res) => {
- if (req.header("x-gateway-verified") !== "1") {
- return res.status(403).send("forbidden");
- }
-
- const target = String(req.header("x-partner-target") ?? "");
- const response = await fetch(target);
- res.status(response.status).send(await response.text());
-});
+router.get("/internal/partner-fetch", async (req, res) => {
+ // Do not rely on a header for access control. Validate requests via a trusted gateway or secure session.
+ // If you still need a gateway check, implement a robust server-side check here (e.g., a signed token).
+ const gatewayOk = req.headers["x-gateway-verified"] === "1"; // kept for backward compatibility only as a soft check
+ if (!gatewayOk) {
+ return res.status(403).send("forbidden");
+ }
+
+ // Validate target against a whitelist. Only allow predefined partner endpoints.
+ const rawTarget = String(req.header("x-partner-target") ?? "");
+ const allowedHosts = [
+ // Add allowed hostnames here, examples:
+ "api.partner.example.com",
+ "services.partner.local",
+ ];
+ let url: URL;
+ try {
+ url = new URL(rawTarget);
+ } catch {
+ return res.status(400).send("invalid target");
+ }
+
+ // Enforce http/https only and ensure host is in allowlist
+ if (url.protocol !== "http:" && url.protocol !== "https:") {
+ return res.status(400).send("unsupported URL scheme");
+ }
+ if (!allowedHosts.includes(url.hostname)) {
+ return res.status(403).send("forbidden target");
+ }
+
+ // Optional: enforce same-origin path restrictions if needed
+ try {
+ const response = await fetch(url.toString());
+ // Relay status and body as-is, but be mindful of large payloads. Consider streaming in real apps.
+ res.status(response.status);
+ const text = await response.text();
+ res.send(text);
+ } catch (err) {
+ res.status(502).send("bad gateway");
+ }
+});
*** End PatchDid we do a good job? 👍 Was helpful, 👎 Needs improvement
Questions about this finding? Tag @vidoc-local in a reply and I'll answer. Specific feedback or suggestions are welcome too!
1 participant
Summary
Results
/repo/prefixProduct bugs found