Review Agent PRs That Mark User JSON Public

The scene is a quiet Friday deploy on a small API. An agent pull request adds one invoice route. The diff touches twelve lines and two tests.

Staging passes those tests in under a minute. A shared cache can then mix two user payloads. One session can show the other user's invoice list.

The new route required a valid bearer token. The handler still set a public cache header. That pair is the defect this review targets.

This review targets HTTP cache headers, not in-process maps. An unbounded process Map is a different defect. Header policy is the only subject in this review.

The diff that looked finished

The generated change followed a familiar handler shape. The handler loaded rows for the current user. It returned JSON with a long max age.

// Agent diff excerpt. Treat as untrusted review input.
app.get('/v1/invoices', requireUser, async (req, res) => {
  const rows = await loadInvoices(req.user.id);
  res.set('Cache-Control', 'public, max-age=300');
  res.json({ rows });
});

The new test called the route only once. It asserted status 200 and a row count. It never sent a second identity through a shared cache.

Neat formatting hid an unclear response privacy contract. The handler looked small beside those new tests. The failure showed up only across two users.

Agents often copy cache headers from static asset examples. Those examples are valid only for public files. The same line is unsafe on a user route.

What a reviewer can trust

Trust the route path if the spec already named it. Trust the query when it binds the user id. Trust a test that fails before the header fix.

Do not trust a lone single-user status check. Do not trust public beside the auth middleware. Do not trust a comment that says the CDN is private.

The RFC 9111 text defines Cache-Control response directives. The public directive permits shared caches to store it. The private directive marks a response for one user.

The no-store directive forbids storing the response at all. A shared cache may ignore Authorization without Vary. That ignore path is the cross-user leak.

Header text alone is not proof of safety. must-revalidate does not split users inside a shared cache. It only constrains reuse after the response becomes stale.

RFC 9111 is the primary source for these directive names.

What to revert before merge

Reviewers revert every public directive on authenticated JSON. Reviewers should revert s-maxage on user payloads. Revert tests that never cross two user identities.

Put the conservative header back on the handler. Keep Vary when any proxy can still cache.

// Proposed replacement. Not executed in this draft.
app.get('/v1/invoices', requireUser, async (req, res) => {
  const rows = await loadInvoices(req.user.id);
  res.set('Cache-Control', 'private, no-store');
  res.set('Vary', 'Authorization, Cookie');
  res.json({ rows });
});

Also revert a helper that maps every 200 to one policy. Auth routes and anonymous routes need different directives. One shared helper caused this exact privacy miss.

Numbered review steps

1. List every route touched by the agent diff.
2. Mark each touched route as anonymous or authenticated.
3. Read every Cache-Control write and every Vary write.
4. Reject public and s-maxage on those authenticated routes.
5. Add a two-identity test in front of a cache.
6. Confirm the second body differs from the first body.
7. Confirm the second status is not a cached 200.
8. Keep production CDN settings out of this fixture.

Proposed two-identity harness

The harness below is only a written proposal. This article did not execute the harness below. Run it only on a local process or an isolated free server.

// Proposed ESM fixture. Unexecuted example.
import express from 'express';

const app = express();
const invoices = {
  'user-a': [{ id: 'inv-a' }],
  'user-b': [{ id: 'inv-b' }],
};

function requireUser(req, res, next) {
  const token = String(req.get('authorization') || '');
  const userId = token.replace('Bearer ', '');
  if (!invoices[userId]) return res.status(401).end();
  req.user = { id: userId };
  next();
}

app.get('/v1/invoices', requireUser, (req, res) => {
  res.set('Cache-Control', 'private, no-store');
  res.set('Vary', 'Authorization');
  res.json({ rows: invoices[req.user.id] });
});

app.listen(3000);

A tiny shared cache sits in front of the app. This proxy is also a proposed unexecuted example. It stores only responses that carry a public directive.

This proxy is a teaching sketch, not a production cache. It ignores status codes, hop headers, and request bodies. Use it only to show a public-header leak.

// Proposed cache proxy. Unexecuted sketch.
import http from 'node:http';

const store = new Map();

function isPublic(header) {
  return /(?:^|,)\s*public\s*(?:,|$)/i.test(header || '');
}

const proxy = http.createServer((req, res) => {
  const key = req.url;
  if (store.has(key)) {
    res.setHeader('X-Cache', 'HIT');
    return res.end(store.get(key));
  }
  const upstream = http.request(
    { hostname: '127.0.0.1', port: 3000, path: req.url, headers: req.headers },
    (up) => {
      const chunks = [];
      up.on('data', (chunk) => chunks.push(chunk));
      up.on('end', () => {
        const body = Buffer.concat(chunks);
        const cc = String(up.headers['cache-control'] || '');
        if (isPublic(cc)) store.set(key, body);
        res.setHeader('X-Cache', 'MISS');
        res.end(body);
      });
    }
  );
  upstream.end();
});

proxy.listen(3001);

Commands for the check stay short and local. They are proposed commands, not a recorded run.

# Proposed commands. Point them at the fixture only.
curl -sD - -o /tmp/a.json -H 'Authorization: Bearer user-a' \
  http://127.0.0.1:3001/v1/invoices
curl -sD - -o /tmp/b.json -H 'Authorization: Bearer user-b' \
  http://127.0.0.1:3001/v1/invoices
cmp /tmp/a.json /tmp/b.json

The cmp tool should report a file difference. A silent match means user B received user A data. That match is a revert signal, not a flake.

A header unit test still belongs in the suite. It catches a public directive before the proxy run. It does not replace the cross-user body check.

Start the fixture process before the header test. The test assumes the fixture process already listens.

// Proposed test. Unexecuted example.
import assert from 'node:assert/strict';
import test from 'node:test';

test('authenticated invoices are not stored as public', async () => {
  const res = await fetch('http://127.0.0.1:3000/v1/invoices', {
    headers: { authorization: 'Bearer user-a' },
  });
  const cc = res.headers.get('cache-control') || '';
  assert.equal(cc.includes('public'), false);
  assert.match(cc, /private|no-store/);
});

Decision table for the header

Route class	Header to keep	Header to reject	Test that must fail first
Authenticated JSON	private, no-store	public, s-maxage	Second identity sees the first body
Anonymous asset	public, max-age=60	blanket no-store	Missing cache hit on a public file
Mixed helper	Split by auth flag	One policy for every 200	Auth route inherits public

The table is a review aid for headers. It is not a CDN vendor compatibility matrix. Vendor defaults still need their own primary docs.

Where an isolated server helps

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

MonkeyCode is the open-source project named for this workflow. The operator supplied that context for the draft. Free model access and a free server are the relevant options.

The operator states that free model access includes a ten-million-token budget. The operator also states that a free server option exists. This page adds no extra quota, duration, or hardware claim.

Those two options fit this review without touching production. The free server can host the fixture app and the proxy. Free model access can draft the failing two-identity test.

The reviewer still owns the final merge call. A model draft is not a passing test run. A token budget does not measure cache correctness.

A reviewer can place this fixture on that free server. A small share of the stated token budget can draft the failing test. The merge decision stays with the human reading the diff.

Limits of this review

This draft records no live benchmark and no vendor trace. Cache products differ on Vary and on Authorization. A green local proxy does not certify an edge network.

The private directive does not encrypt the body in transit. The no-store directive does not replace access control. TLS, auth, and query binding remain separate checks.

The ten-million-token figure is only an operator-stated budget. It is not a measurement from this article. Duration, model names, and hardware stay unspecified here.

Do not treat those blanks as product promises. Agent diffs can omit Vary while copying a private header. Read both header lines on every auth route.

A downstream override can still leak a private response. Compare the origin header with the edge response. Treat a mismatch as a failed review, not a pass.

Who should skip this approach

Skip the proxy proof when no shared cache exists. The header rule can still land in that case. The two-curl story would overclaim the real path.

Skip a blanket no-store policy on public docs. Those routes want a measured public cache policy. Copying the invoice header there wastes origin capacity.

Skip approval when two identities cannot be minted. Header inspection without a cross-user request is incomplete. Wait for the harness before merging the route.

Skip this method for streaming downloads and range responses. Those responses need a separate cache review path. This article covers small authenticated JSON bodies only.

Closing the review

Trust a bound user id and a failing cross-user test. Remove every public cache directive from authenticated JSON. Exercise a shared cache before approving the merge.

The agent can still write the route handler. The reviewer still owns the response privacy contract. A short harness makes that contract visible in review.