DEV Community

Casey Chen
Casey Chen

Posted on

A Free Endpoint Is Not an Allowlist: Reviewing Agent PRs That Add Fetch

An agent pull request that switches a client onto a free model server is three changes, not one. The client binding, the live smoke, and any new outbound tool each move data across a trust boundary. Review them separately. Trust a hunk only when a local test proves the bound. Revert the live path when that proof is missing.

The specimen in this note is constructed for review practice. It is not a report of a merged repository, a production incident, or a measured product limit.

What the specimen changes

The diff adds four surfaces that look unrelated in a file list:

  1. agent/client.py defaults base_url to a hosted model endpoint and reads a key from the environment.
  2. agent/tools/fetch.py registers a tool that issues GET for a URL chosen by the model.
  3. .github/workflows/model-smoke.yml runs the client on push and pull_request.
  4. tests/test_client.py expects a live completion instead of a stub.

Each file can be justified in isolation. The combination sends prompt text, tool traffic, and CI identity onto a network path the repository did not have. That combination is the review subject.

A short diff stat is not evidence of a small risk. Forty lines that open egress outweigh four hundred lines of renaming.

Gate table

Use the table as a merge gate. A row that fails its proof column blocks the PR even when the other rows look tidy.

Hunk Trust only if Revert if Proof
Client base URL Default is empty or loopback, and hosted URLs arrive from outside config Library code defaults to a hosted endpoint Construct the client with an empty environment and assert no socket use
Credential read The key stays in the runner secret store A key, .env, or Authorization value is in the diff Search the diff for API_KEY=, sk-, and Authorization
Fetch tool Allowlist is executable code, and redirects are off Any URL, file:, or link-local target is accepted Table tests for allow and deny, including redirects
CI smoke Trigger is workflow_dispatch only, prompt is canned, timeout is set Trigger is push, pull_request, or schedule Read the on: block; run tests with the network stubbed
Tests Default suite makes no network call Passing requires a live server Run the suite with sockets disabled

Review order

Read in this order so a client rename cannot hide the permission change.

  1. Tool schema and HTTP helper.
  2. Workflow triggers, then steps.
  3. Tests, looking for live hosts and HTTP libraries.
  4. Client defaults last.

A clean client does not redeem an open tool. A green live test does not redeem an open tool either. The live test is the policy you are trying to avoid making mandatory.

Commands

git diff --stat origin/main...HEAD
git diff origin/main...HEAD -- agent/tools .github/workflows tests
rg -n "httpx|requests\\.|base_url|workflow_dispatch|pull_request" agent .github tests
Enter fullscreen mode Exit fullscreen mode

If the search hits both a fetch tool and a workflow trigger, treat the diff as one policy change. The model identifier string is the least useful line in the file.

Proposed allowlist

The module below is a proposed review gate for the specimen. It is not a benchmark, and it was not executed for this article.

from urllib.parse import urlparse

ALLOWED_HOSTS = frozenset({"docs.example.com", "static.example.com"})

class EgressDenied(Exception):
    pass

def assert_fetch_allowed(url: str) -> str:
    parsed = urlparse(url)
    if parsed.scheme != "https":
        raise EgressDenied(f"scheme not allowed: {parsed.scheme!r}")
    host = (parsed.hostname or "").lower()
    if host not in ALLOWED_HOSTS:
        raise EgressDenied(f"host not allowed: {host!r}")
    if parsed.username or parsed.password:
        raise EgressDenied("credentials in URL")
    return parsed.geturl()
Enter fullscreen mode Exit fullscreen mode

The matching tests are the artifact a reviewer can ask the agent to keep. They fail closed.

import pytest

@pytest.mark.parametrize("url", [
    "https://docs.example.com/guide",
    "https://static.example.com/a.png",
])
def test_allows_known_hosts(url):
    assert assert_fetch_allowed(url) == url

@pytest.mark.parametrize("url", [
    "http://docs.example.com/guide",
    "https://user:pw@docs.example.com/x",
    "https://169.254.169.254/latest/meta-data/",
    "https://evil.example/docs.example.com",
    "file:///etc/passwd",
    "https://docs.example.com.evil.example/x",
])
def test_denies_unlisted_targets(url):
    with pytest.raises(EgressDenied):
        assert_fetch_allowed(url)
Enter fullscreen mode Exit fullscreen mode

Two gaps belong in the review comment, not in a follow-up ticket that never lands. The function does not follow redirects, so the HTTP client must disable redirect following or re-check every hop. The function does not pin DNS, so an allowed name can still resolve somewhere unexpected. If the team cannot accept both gaps, remove the tool.

Comments are not the gate

A docstring that says not to fetch internal hosts is not an allowlist. A PR description that calls the smoke safe because the server is free is not a trigger restriction. Ask for the test file. If the agent updates the comment and leaves the tool open, revert the tool.

Where a free server is allowed to appear

Disclosure: This article was prepared as part of MonkeyCode's product outreach.

For this draft, MonkeyCode is relevant only as an operator-supplied option: a free model access path and a free server option are availability claims, not measured limits. This note does not state model names, token quotas, hardware, duration, uptime, or price. Those facts change, and none of them were verified here. Check the current primary terms before use. A free path is still an external processor of whatever you send.

Keep the hosted call inside all four constraints:

  • Default client target stays empty or loopback. Hosted configuration is injected, not committed.
  • The only workflow trigger is manual, with an explicit boolean input.
  • The prompt file is canned. It contains no source, no secrets, and no user data.
  • The job stores status, not raw prompt or completion text. Timeout is short. Retries stay at zero until a separate review.
# Proposed fragment for review. Not a deployed workflow.
on:
  workflow_dispatch:
    inputs:
      run_live_smoke:
        description: Run one canned call against the hosted endpoint
        required: true
        type: boolean
jobs:
  smoke:
    if: ${{ inputs.run_live_smoke == true }}
    timeout-minutes: 5
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v4
      - name: Canned prompt only
        run: python -m agent.smoke --prompt-file ci/canned_prompt.txt
Enter fullscreen mode Exit fullscreen mode

Revert on: push and on: pull_request from the specimen. A free endpoint still receives the prompt, still shares capacity, and still fails for reasons that have nothing to do with the change under review. Making that call required on every push converts a convenience into a merge dependency.

If the free server is down, skip the smoke. Do not fall back to an unbounded fetch, and do not fall back to a second hosted vendor inside the same unreviewed hunk. Fallback is a new data flow. It needs its own row in the gate table.

Failures a green unit test can hide

  • The allowlist is strict, and the same PR appends a host for a demo. Review allowlist edits like auth edits. A one-line host addition is a permission grant.
  • The smoke is manual, and the step still prints the response body. Log retention is a copy. Print a status code and a byte length, not the text.
  • The application allowlist is correct, and HTTPS_PROXY is set in the runner. Proxy configuration can move bytes after your check. Fix the runner environment, or deny the run when a proxy is present.
  • A retry wrapper lands beside the smoke. Five retries are five sends. The smoke job should set the retry count to zero.
  • The test suite mocks assert_fetch_allowed itself. That test proves the mock, not the gate. Call the real function.

Who should skip this split

Do not use the manual-smoke pattern when any item below is true.

  • Prompts can contain customer data, credentials, or unreleased source. Keep that traffic off hosted endpoints, free or paid.
  • Nobody on the reviewing team can read the tool schema. An unread schema is an unread permission. Close the tool until a human can name every argument.
  • Redirects cannot be disabled, and the host list cannot be kept explicit. Delete the fetch tool. A warning comment is not a substitute.
  • The goal is a latency or quality number. This protocol does not produce one. It produces a pass or fail on egress and on trigger scope.

Approval check

Approve only when four facts are visible in the diff: a local or empty client default, deny-by-default fetch tests, no live call on push or pull request, and no secret material. The hosted free-server option can remain as a manual canned smoke after those four are true. If one is missing, land the allowlist and revert the live path.

When a similar agent PR is already in the queue, run the deny cases against a stub before enabling any hosted smoke. The endpoint that answers is not the review. The bytes you allow to leave are the review.

Top comments (0)