Skip to content

Implement a local MCP server to calculate the final line numbers of the security findings - #41

Merged
heltonduarte merged 2 commits into
mainfrom
line-numbers
Sep 30, 2025
Merged

Implement a local MCP server to calculate the final line numbers of the security findings#41
heltonduarte merged 2 commits into
mainfrom
line-numbers

Conversation

@heltonduarte

Copy link
Copy Markdown
Contributor

This MCP is a starting point for several other functionalities that can be added to the extension.

Comment thread commands/security/analyze.toml Outdated

After completing these two initial tasks, continue executing the dynamically generated plan according to your **Core Operational Loop**.

**Final Report Generation**

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this go in the GEMINI.md with other reporting instructions? interested in @shrishabh thoughts as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the GEMINI.md file.

Comment thread gemini-extension.json Outdated
"securityServer": {
"command": "node",
"args": ["${extensionPath}/mcp-server/security.ts"],
"env": {}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe remove this for now

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Comment thread gemini-extension.json Outdated
"name": "gemini-cli-security",
"version": "0.1.0",
"contextFileName": "GEMINI.md"
"version": "0.2.0",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you keep this at 0.1.0 until we have Github releases (hopefully soon)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reverted.

Comment thread mcp-server/security.ts Outdated
const safeFilePath = await dependencies.fs.realpath(
dependencies.path.resolve(CWD, filePath)
);
if (!safeFilePath.startsWith(CWD)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: can you double-check whether CWD contains the trailing separate i.e. /foo/bar/ ... otherwise this isn't an adequate check e.g. /foo/bar vs a similarly named /foo/barbar

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Added a path.sep at the end: https://nodejs.org/api/path.html#pathsep

Comment thread mcp-server/security.ts Outdated
dependencies.path.resolve(CWD, filePath)
);
if (!safeFilePath.startsWith(CWD)) {
throw new Error('File path is outside of the current working directory.');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

here it throws errors, but elsewhere it "returns" the error. any reason why?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Returning the error here as well.

Comment thread mcp-server/security.ts
if (!safeFilePath.startsWith(CWD)) {
throw new Error('File path is outside of the current working directory.');
}
const content = await dependencies.fs.readFile(safeFilePath, 'utf-8');

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it should return an explicit error fit he file does not exist e.g. to try to help the LLM if it makes a mistake. not entirely sure if it's needed but worth thinking about.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We do end up returning a file not found error.

@anguillanneuf

Copy link
Copy Markdown
Collaborator

@heltonduarte is this tool find_line_numbers ever meant to be explicitly called by the user? can the user invoke this tool if they try and if so how?

I imagine find_line_numbers is like an internal tool that only the extension/agent calls.

@heltonduarte

Copy link
Copy Markdown
Contributor Author

@heltonduarte is this tool find_line_numbers ever meant to be explicitly called by the user? can the user invoke this tool if they try and if so how?

I imagine find_line_numbers is like an internal tool that only the extension/agent calls.

This tool is only to be invoked by the extension at the end of the security report generation. It's not meant for users to invoke it directly.

@heltonduarte
heltonduarte merged commit ae9ffd6 into main Sep 30, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants