fix: AppSec host allowlists (SSRF / contribute POST)#9
Conversation
Consent no longer implies destination trust. Outbound sessions disable redirects and ambient proxy env. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 909d65e888
ℹ️ 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".
| if host_allowlist is not None: | ||
| return [h.strip() for h in host_allowlist if h and h.strip()] |
There was a problem hiding this comment.
Merge env hosts when a fetch allowlist is supplied
When SCP_ANTIGEN_FETCH_HOST_ALLOWLIST=example.com is set and an MCP/SDK caller also supplies allowlist with an issuer pubkey or other non-host entry, this branch returns only the caller-provided list and drops the env hosts. Because host_allowed() ignores 64-hex entries, a fetch of https://example.com/... is rejected with host_not_on_allowlist, despite the tool contract saying hosts can come from allowlist and/or SCP_ANTIGEN_FETCH_HOST_ALLOWLIST. Merge the two sources or filter/union host entries so adding an issuer allowlist does not disable the configured host allowlist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 909d65e888
ℹ️ 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".
| # Synced with OpenHarness docs/contracts/scp_antigen_mcp_v1.md | ||
| EXPECTED_SCP_ANTIGEN_MCP_V1_SHA256 = ( | ||
| "63e2e313d70e738119a0878860a172205158f8113b08690fc1aec1e4aed76ec6" | ||
| "bc44c12956fe1c819220b02069ead357ccea0ca3b72774d47ec12590a965c64b" |
There was a problem hiding this comment.
Update antigen contract hash to the actual digest
For the changed docs/contracts/scp_antigen_mcp_v1.md, hashlib.sha256(path.read_bytes()).hexdigest() now returns a86ac10d45a4812a045a0fb7ddedeaf86fa100da6131a347b2485ac673bee20e, not the new value here. In any environment that runs test_vendored_scp_antigen_mcp_v1_contract_sha256, this expectation fails immediately, so the contract hash update does not match the document it is meant to pin.
Useful? React with 👍 / 👎.
Summary
SCP_ANTIGEN_FETCH_HOST_ALLOWLIST/ MCP host allowlist before antigen HTTPS fetchSCP_CONTRIBUTE_HOST_ALLOWLISTbefore contribute POST (even with consent)allow_redirects=False,trust_env=Falsehttp_policyhelper; docs + contract hash updatedTest plan
PYTHONPATH=src pytest -q tests/test_appsec_host_allowlists.pyand related suitesCIgreen