fix(security): close CWE-939 urlopen sinks in Cursor memory CLIs - #225
Merged
Conversation
Memory and Neo4j clients now use a socket HTTP helper that refuses file:// and unsigned remote http, keeping the documented C1 and loopback paths. Co-authored-by: Cursor <cursoragent@cursor.com>
|
📋 Best Practices for Large Changes
✅ This PR passes the blocking limit but is larger than recommended. |
L9 Audit Harness Report
Step Results
Architecture Audit Findings
See Spec Coverage
See Next StepsAll checks passed. Safe to merge. |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: abd7670d34
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
CI on this branch was red on ruff-format, and CodeQL raised a new high
alert against the socket client the PR introduces. Root causes, all in
code this PR touches:
- The shared SSL contexts came from a bare ssl.create_default_context(),
which still negotiates TLSv1 and TLSv1.1. Both contexts now come from
secure_ssl_context(), pinned to TLSv1_2 as the minimum.
Closes CodeQL "Use of insecure SSL/TLS version" (py/insecure-protocol).
- require_http_url embedded the first 120 characters of the rejected URL
in every error message, so a userinfo-bearing endpoint leaked its
credentials into any log that recorded the error. Messages now carry
redact_url() output: scheme and authority only.
- The Host header was built as f"{host}:{port}", which produced the
invalid authority "::1:8000" for an IPv6 loopback URL, and it dropped
an explicit port whenever it was 80 or 443 regardless of scheme, so
https://host:80 was sent as "Host: host". format_authority() brackets
IPv6 literals, and the port is now omitted only when it is the default
for the selected scheme.
- ruff format over the two files CI flagged.
Tests cover each fix: minimum TLS version on all three live contexts,
userinfo never surfacing in an error, redact_url dropping path/query and
bracketing IPv6, format_authority across the four host/port shapes, and
a real loopback exchange asserting the Host header keeps its port.
Local: ruff check + ruff format --check clean on 0.15.5 and 0.15.8,
mypy engine/ clean (139 files), pytest -m unit 741 passed / 3 pre-existing
skips.
Remediation-Cycle: Cognitive.Engine.Graphs#225/cycle-1
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015dio7eWN7aBnLnszqmF2JS
CodeQL raised py/incomplete-url-substring-sanitization (high) against the new userinfo test on the previous head: `assert "example.com" in message` reads as a host check that a crafted URL could satisfy at an arbitrary position. The assertion is stronger as an equality anyway — redact_url() output is fully determined, so the test now pins the whole message. No suppression and no lost coverage: the credential-absence assertions are unchanged. Remediation-Cycle: Cognitive.Engine.Graphs#225/cycle-2 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015dio7eWN7aBnLnszqmF2JS
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
urllib.request.urlopenincursor_memory_client.pyandcursor_neo4j_query.py(CWE-939 /file://).file://, userinfo, and unsigned remote HTTP. Documented C1http://46.62.243.82and loopback still work.requestsdependency.Test plan
pytest tests/unit/test_cursor_memory_http.py --noconftestruff checkon the four changed filesMade with Cursor