Fix A001: rename id shadow variables in jsonrpyc#197
Conversation
….toml Agent-Logs-Url: https://github.com/GitHubSecurityLab/seclab-taskflow-agent/sessions/fcbe7b59-b518-4185-a679-b6cf70c6a3bb Co-authored-by: kevinbackhouse <4358136+kevinbackhouse@users.noreply.github.com>
id shadow variables in jsonrpyc
There was a problem hiding this comment.
Pull request overview
This PR addresses Ruff rule A001 by removing local variable assignments that shadow the Python built-in id in the CodeQL JSON-RPC helper, and then tightens linting by un-ignoring A001 in the repo’s Ruff configuration.
Changes:
- Refactor
Spec.request,Spec.response, andSpec.errorto avoid reassigning theidparameter (introduceencoded_id). - Rename the local
idvariable toreq_idthroughoutRPC.call. - Remove
"A001"from Ruff’s ignore list inpyproject.toml.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/seclab_taskflow_agent/mcp_servers/codeql/jsonrpyc/__init__.py |
Eliminates id shadowing by renaming locals and introducing encoded_id for message building. |
pyproject.toml |
Stops globally ignoring Ruff A001 now that the known violations are addressed. |
Comments suppressed due to low confidence (2)
src/seclab_taskflow_agent/mcp_servers/codeql/jsonrpyc/init.py:188
- Same issue as in
request(): only string IDs are JSON-encoded. Becausecheck_idacceptsint(and thusbool), a boolean ID would be emitted asTrue/Falsehere, which is invalid JSON. Preferencoded_id = json.dumps(id)(or reject bool incheck_id) to ensure the response is always valid JSON.
encoded_id = json.dumps(id) if isinstance(id, str) else id
# build the response string
try:
res = f'{{"jsonrpc":"2.0","id":{encoded_id},"result":{json.dumps(result)}}}'
src/seclab_taskflow_agent/mcp_servers/codeql/jsonrpyc/init.py:237
- Only string IDs are JSON-encoded before being interpolated into the error JSON. Since
check_idallowsint(and thusbool), passingTrue/Falseas an ID would produce invalid JSON. Encoding IDs withjson.dumps(id)unconditionally here (or rejecting bool incheck_id) would avoid this edge case.
encoded_id = json.dumps(id) if isinstance(id, str) else id
# start building the error string
err = f'{{"jsonrpc":"2.0","id":{encoded_id},"error":{err_data}}}'
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| encoded_id = json.dumps(id) if isinstance(id, str) else id | ||
| req += f',"id":{encoded_id}' |
There was a problem hiding this comment.
encoded_id is only JSON-encoded for string IDs, which means a boolean ID (allowed by check_id because bool is a subclass of int) would be rendered as True/False in the JSON output, producing invalid JSON. Consider encoding IDs with json.dumps(id) unconditionally here (or tightening check_id to reject bool) so the emitted JSON is always valid.
This issue also appears in the following locations of the same file:
- line 184
- line 234
See below for a potential fix:
# always JSON-encode the id to ensure valid JSON (e.g. for booleans)
encoded_id = json.dumps(id)
Ruff rule A001 flags local variable assignments that shadow Python built-ins. All five violations were in
mcp_servers/codeql/jsonrpyc/__init__.py, whereid(a built-in) was used as a local variable name.Changes
Spec.request/Spec.response/Spec.error: Replace theid = json.dumps(id)reassignment pattern with a newencoded_idvariable:RPC.__call__: Rename the localidvariable toreq_idthroughout the method (assignment, dict lookups, and return value).pyproject.toml: Remove"A001"from[tool.ruff.lint] ignore.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
unsupported.example.com/usr/bin/python python -m pytest tests/ -x -q(dns block)If you need me to access, download, or install something from one of these locations, you can either: