[SCAL-319676] changes for search_objects and fetch_data tools, along with local instance deployment - #177
[SCAL-319676] changes for search_objects and fetch_data tools, along with local instance deployment#177saharsh-ts wants to merge 4 commits into
search_objects and fetch_data tools, along with local instance deployment#177Conversation
⛔ Snyk checks have failed. 1 issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
![snyk-io[bot]](https://avatars.githubusercontent.com/in/372950?s=60&v=4){kind=small}
| ctx.setChosenInstanceUrl(sanitized); | ||
|
|
||
| res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" }); | ||
| res.end(renderManualPage(sanitized, ctx.nonce)); |
There was a problem hiding this comment.
Cross-site Scripting (XSS)
Unsanitized input from the request URL flows into end, where it is used to render an HTML page returned to the user. This may result in a Cross-Site Scripting attack (XSS).
Line 145 | CWE-79 | Priority score 810 | Learn more about this vulnerability
Data flow: 19 steps
Step 1 - 6
mcp-server/src/local-auth/browser-login.ts
Line 222 in a7378ae
Step 7 - 13 src/local-auth/browser-login.ts#L223
Step 14 - 15 src/local-auth/browser-login.ts#L225
Step 16 src/local-auth/browser-login.ts#L138
Step 17 - 19
mcp-server/src/local-auth/browser-login.ts
Line 145 in a7378ae
⚡ Refresh the page to see if a fix suggestion is available 🔄
There was a problem hiding this comment.
⚡ Snyk Agent Fix suggestion 1 of 1
Snyk Agent Fix explanation is not available
Code changes
--- src/local-auth/browser-login.ts
+++ src/local-auth/browser-login.ts
@@ -141,8 +141,15 @@
}
ctx.setChosenInstanceUrl(sanitized);
+ const parsedOrigin = new URL(sanitized).origin;
+ if (!/^https?:\/\/[A-Za-z0-9.\-]+(:\d+)?$/.test(parsedOrigin)) {
+ res.writeHead(400);
+ res.end("Invalid cluster URL");
+ return;
+ }
+ const safeInstanceUrl = parsedOrigin;
res.writeHead(200, { "Content-Type": "text/html; charset=utf-8" });
- res.end(renderManualPage(sanitized, ctx.nonce));
+ res.end(renderManualPage(safeInstanceUrl, ctx.nonce));
return;
}
Content generated by AI, expires on 2026-07-09 07:26:32 UTC. Refresh the page after running Snyk commands.
Commands
- ✅ To apply this fix and create a commit - reply with
@snyk /apply 1
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a local browser-based authentication flow for the stdio deployment of the ThoughtSpot MCP server, including on-disk credential caching. It also adds two new tools, search_objects and fetch_data, to enable full-text metadata searching and tabular data fetching for Answers and Liveboards, supported by a terminology synonym glossary. Feedback on these changes highlights a potential Denial of Service vulnerability in readBody due to unbounded request stream buffering, and a pagination bug where duplicate results can be returned when post-filters are active.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function readBody(req: IncomingMessage): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| let data = ""; | ||
| req.on("data", (chunk) => { | ||
| data += chunk; | ||
| }); | ||
| req.on("end", () => resolve(data)); | ||
| req.on("error", reject); | ||
| }); | ||
| } |
There was a problem hiding this comment.
The readBody function reads the entire request stream into memory without any size limit. Since this is a local HTTP server, an excessively large payload could lead to memory exhaustion (DoS). It is highly recommended to enforce a reasonable maximum payload size limit (e.g., 1MB) and abort the request if it exceeds the limit.
function readBody(req: IncomingMessage): Promise<string> {
return new Promise((resolve, reject) => {
let data = "";
const maxLimit = 1024 * 1024; // 1MB limit
req.on("data", (chunk) => {
data += chunk;
if (data.length > maxLimit) {
req.destroy();
reject(new Error("Payload too large"));
}
});
req.on("end", () => resolve(data));
req.on("error", reject);
});
}| // Enforce limit; on overshoot point the cursor back at the dropped | ||
| // matches' page (duplicates possible, skips never). | ||
| let next_cursor = hasMorePages ? String(pageOffset) : null; | ||
| if (objects.length > limit) { | ||
| objects = objects.slice(0, limit); | ||
| next_cursor = String(pageOffset - limit); | ||
| } |
There was a problem hiding this comment.
When post-filters (like owner, tag, or modifiedSince) are active and we overshoot the limit across multiple pages, pointing the cursor back to the start of the last page (pageOffset - limit) causes the matches that were already returned in the current page to be returned again in the next page, leading to duplicate results. To achieve duplicate-free and skip-free pagination, consider encoding both the offset and the number of items to skip in the cursor (e.g., offset,skip).
There was a problem hiding this comment.
patches/@thoughtspot+mcp-auth+1.0.0.patch should not be there
2 participants
No description provided.