Full TDX integration#174
Conversation
…er functionality next sprint)
118 tdx part 1
127 tdx pt 2
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| const ticketRes = await fetch(`${process.env.TDX_URL}/${process.env.TDX_APP_ID}/tickets/${ticketId}`, { | ||
| headers: { | ||
| 'Authorization': `Bearer ${token}`, | ||
| 'Content-Type': 'application/json' | ||
| } | ||
| }); |
Check failure
Code scanning / CodeQL
Server-side request forgery Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, the user-controlled ticketId must be constrained so it cannot arbitrarily alter the URL path. The safest approach without changing functionality is to validate ticketId against a strict allow‑list pattern (for example, only allow alphanumeric characters and a few safe symbols) and reject requests that do not match. This ensures the request will always target /tickets/<well‑formed-id> and prevents path traversal or additional path segment injection.
Concretely, in api/index.js inside the /api/tdx-ticket route:
- After checking that
ticketIdis present (lines 417–419), add a format validation step, e.g., a regular expression such as/^[A-Za-z0-9_-]+$/(or a stricter pattern if you know the exact ticket format). IfticketIdfails this check, respond with400 Bad Request. - Use the validated
ticketIdwhen constructing the URL in thefetchcall (line 422). Since we’re only validating and not transforming the value, existing successful use cases will continue to work as long as the IDs are well‑formed.
No new methods or external libraries are needed; we can rely on native RegExp and simple checks in the same file and function.
| @@ -418,8 +418,15 @@ | ||
| return res.status(400).json({ error: "Ticket ID is required" }); | ||
| } | ||
|
|
||
| // Validate ticketId to prevent SSRF/path manipulation | ||
| const ticketIdString = String(ticketId); | ||
| const ticketIdPattern = /^[A-Za-z0-9_-]+$/; | ||
| if (!ticketIdPattern.test(ticketIdString)) { | ||
| return res.status(400).json({ error: "Invalid Ticket ID format" }); | ||
| } | ||
|
|
||
| try { | ||
| const ticketRes = await fetch(`${process.env.TDX_URL}/${process.env.TDX_APP_ID}/tickets/${ticketId}`, { | ||
| const ticketRes = await fetch(`${process.env.TDX_URL}/${process.env.TDX_APP_ID}/tickets/${ticketIdString}`, { | ||
| headers: { | ||
| 'Authorization': `Bearer ${token}`, | ||
| 'Content-Type': 'application/json' |
| console.log(`${res.status(500)}: SSO Success, but no token found in HTML output.`); | ||
| } | ||
| } catch (error) { | ||
| res.status(500).send(error); |
Check warning
Code scanning / CodeQL
Information exposure through a stack trace Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
In general, to fix this problem you should avoid sending raw error objects or stack traces to the client. Instead, log the detailed error information on the server (e.g., using console.error or a logging library), and send a generic, user-friendly error message with an appropriate HTTP status code.
For this specific code, the best fix that preserves existing functionality is to update the /api/tdx-auth route’s catch block (lines 405–407). Instead of res.status(500).send(error);, log the error server-side (if not already logged) and send a generic error message such as "Internal Server Error" or a more domain-specific but non-revealing message. The rest of the behavior (status code 500, route logic) remains unchanged. Concretely:
- In
api/index.js, within the/api/tdx-authroute, modify thecatch (error)block to:- Call
console.error("TDX Auth Error:", error);(or similar) to keep detailed diagnostics on the server. - Respond with
res.status(500).send("Internal Server Error");(or equivalent) instead of sendingerrordirectly.
- Call
No new imports or external dependencies are needed; console is globally available in Node.
| @@ -403,7 +403,8 @@ | ||
| console.log(`${res.status(500)}: SSO Success, but no token found in HTML output.`); | ||
| } | ||
| } catch (error) { | ||
| res.status(500).send(error); | ||
| console.error("TDX Auth Error:", error); | ||
| res.status(500).send("Internal Server Error"); | ||
| } | ||
| }); | ||
|
|
| console.log(`${res.status(500)}: SSO Success, but no token found in HTML output.`); | ||
| } | ||
| } catch (error) { | ||
| res.status(500).send(error); |
Check warning
Code scanning / CodeQL
Exception text reinterpreted as HTML Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 3 months ago
To fix the problem, avoid sending the raw error object (or its full, unsanitized string representation) directly in the HTTP response. Instead, send a safe, generic error message to the client, and log the detailed error server-side for debugging. If you need to expose details, restrict them to non-user-controlled parts or properly escape them.
Concretely, in api/index.js within the /api/tdx-auth route’s catch block (lines 405–407), replace res.status(500).send(error); with a safe response such as res.status(500).send("Internal Server Error"); (or a JSON body with a generic error message), and additionally log error using console.error or an existing logging mechanism. This preserves functionality (the endpoint still returns 500 on failure) while eliminating the possibility of reflected XSS through exception text. No new imports or dependencies are required; we can use the existing console for logging.
| @@ -403,7 +403,8 @@ | ||
| console.log(`${res.status(500)}: SSO Success, but no token found in HTML output.`); | ||
| } | ||
| } catch (error) { | ||
| res.status(500).send(error); | ||
| console.error("TDX SSO Error:", error); | ||
| res.status(500).send("Internal Server Error"); | ||
| } | ||
| }); | ||
|
|
3 participants
Working PR for all TDX implementation phases