merge main: scorecard hardening and the fetch/render bounding fixes

.github/workflows/ci.yml

@@ -14,8 +14,8 @@ jobs:
   test:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v7
-      - uses: actions/setup-node@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
         with:
           node-version: 24
       - run: npm ci

.github/workflows/codeql.yml

@@ -10,6 +10,9 @@ on:
   schedule:
     - cron: "17 3 * * 1"
 
+# The analyze job widens its own permissions; everything else gets none.
+permissions: read-all
+
 jobs:
   analyze:
     name: Analyze
@@ -20,12 +23,12 @@ jobs:
       security-events: write
 
     steps:
-      - uses: actions/checkout@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
 
-      - uses: github/codeql-action/init@v3
+      - uses: github/codeql-action/init@42947a340483f03ba47bb1a039b2c519aab3df85 # v3.37.8
         with:
           languages: javascript-typescript
 
-      - uses: github/codeql-action/analyze@v3
+      - uses: github/codeql-action/analyze@42947a340483f03ba47bb1a039b2c519aab3df85 # v3.37.8
         with:
           category: "/language:javascript-typescript"

.github/workflows/dependency-review.yml

@@ -13,5 +13,5 @@ jobs:
   dependency-review:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v7
-      - uses: actions/dependency-review-action@v4
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
+      - uses: actions/dependency-review-action@2031cfc080254a8a887f58cffee85186f0e49e48 # v4.9.0

.github/workflows/publish.yml

@@ -35,15 +35,15 @@ jobs:
     outputs:
       channel: ${{ steps.channel.outputs.channel }}
     steps:
-      - uses: actions/checkout@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
       # No registry-url here: it writes an .npmrc auth-token line with a
       # placeholder value, and npm then authenticates with that instead of
       # falling through to OIDC trusted publishing.
-      - uses: actions/setup-node@v7
+      - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
         with:
           node-version: 24
-      # Trusted publishing needs npm 11.5.1 or newer.
-      - run: npm install -g npm@latest
+      # Trusted publishing needs npm 11.5.1 or newer; node 24 has bundled a
+      # new-enough npm since 24.4, so nothing extra is installed here.
       - run: npm ci
       - run: npm test
       - name: pick channel and version
@@ -103,7 +103,7 @@ jobs:
     if: needs.publish.outputs.channel == 'latest'
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/setup-node@v7
+      - uses: actions/setup-node@820762786026740c76f36085b0efc47a31fe5020 # v7.0.0
         with:
           node-version: 24
       # Same invocation the install-loop experiment proved out, telemetry left

.github/workflows/scorecard.yml

@@ -22,7 +22,7 @@ jobs:
       actions: read
 
     steps:
-      - uses: actions/checkout@v7
+      - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
         with:
           persist-credentials: false
 
@@ -32,12 +32,12 @@ jobs:
           results_format: sarif
           publish_results: true
 
-      - uses: actions/upload-artifact@v4
+      - uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4.6.2
         with:
           name: SARIF file
           path: results.sarif
           retention-days: 5
 
-      - uses: github/codeql-action/upload-sarif@v3
+      - uses: github/codeql-action/upload-sarif@42947a340483f03ba47bb1a039b2c519aab3df85 # v3.37.8
         with:
           sarif_file: results.sarif

SECURITY.md

@@ -0,0 +1,24 @@
+# Security
+
+## Reporting a vulnerability
+
+Report vulnerabilities privately through GitHub: [Security > Report a
+vulnerability](https://github.com/only-cli/oc/security/advisories/new).
+Please do not open a public issue for anything exploitable.
+
+Expect an acknowledgement within a week. Fixes ship as a patch release,
+and the advisory is published once the fix is out.
+
+## Scope
+
+oc fetches untrusted web pages by design, so the interesting bugs are the
+ones where page content escapes its role as data: rendered text that can
+alter what an agent executes, URLs that reach private or internal hosts
+despite the SSRF guard, or a crafted page that breaks the distiller. Bugs
+in the experiments/ directory are out of scope; nothing there ships in
+the package.
+
+## Supported versions
+
+Only the latest release on npm is supported. There is no backporting; a
+security fix means a new release.

experiments/skills-install-remove-loop/Dockerfile

@@ -1,4 +1,4 @@
-FROM node:24-bookworm-slim
+FROM node:24-bookworm-slim@sha256:3638d9a6fe4030bd716be989438248074489337ba3275657f93595428be4fc03
 
 RUN apt-get update \
     && apt-get install --yes --no-install-recommends ca-certificates git \

src/act.js

@@ -136,6 +136,15 @@ export function read(n, { session = DEFAULT_SESSION, budget = 2000 } = {}) {
     if (!line) continue;
     const cost = estimateTokens(line) + 1;
     if (spent + cost > budget && lines.length) break;
+    // The first line always prints so read never answers with nothing, but
+    // its text is the page's to write and so has no natural size. Alone over
+    // budget it still gets cut: 'up to N tokens' is a promise the page must
+    // not be able to break.
+    if (!lines.length && cost > budget) {
+      lines.push(`${line.slice(0, budget * 4)} ... cut at ~${budget} tokens, raise --budget for the rest`);
+      spent += budget;
+      continue;
+    }
     spent += cost;
     lines.push(line);
   }

src/cli.js

@@ -193,8 +193,12 @@ async function main() {
         // Only the blank case here. `raw` is the fallback the compact view's
         // failure line names, so it must not fail on the same pages: a page
         // whose only text is its menu still has markup, and printing it is the
-        // whole point of `raw`.
-        if (outTokens < MIN_CONTENT) noContent(finalUrl, `~${outTokens} tokens of markdown`, '');
+        // whole point of `raw`. And a short page that arrived short is not
+        // blank, so the verdict needs the same evidence the compact view asks
+        // for: near-nothing distilled out of markup that promised more.
+        if (outTokens < MIN_CONTENT && contentFailure(outTokens, htmlTokens)) {
+          noContent(finalUrl, `~${outTokens} tokens of markdown`, '');
+        }
         return;
       }
       const page = distill(html, finalUrl);

src/fetch.js

@@ -35,6 +35,29 @@ const PROXY_TIMEOUT_MS = 300_000;
 // pages of mojibake an agent then pays for, so it is refused by name instead.
 const READABLE_TYPE = /^\s*(?:text\/|application\/(?:json|xml|javascript|x-ndjson|[\w.+-]*\+(?:json|xml)))/i;
 
+// The whole decoded body is buffered before the distiller sees it, so an
+// unbounded response is an unbounded allocation, and a URL is often the
+// page's to name, not the caller's. The cap is generous because oc fetches
+// some large corpora on purpose (the Node.js docs reference is 8.5MB
+// decoded); three times that and a response is not a page anyone reads.
+// Content-Length rejects a known-large response before its bytes arrive, but
+// the header is optional and untrusted, so every transport also counts what
+// actually lands, after decoding, which is what stops a decompression bomb.
+export const MAX_BODY = 25 * 1024 * 1024;
+
+/**
+ * Refuse a body larger than oc will buffer. Called on the Content-Length
+ * header first and again on the bytes as they arrive, since only the second
+ * count is trustworthy.
+ * @param {number} size - bytes seen so far, or claimed by the header
+ * @param {string} url
+ */
+export function assertBodySize(size, url) {
+  if (size > MAX_BODY) {
+    throw new Error(`response body over ${MAX_BODY / 1048576}MB for ${url}, more than oc will read`);
+  }
+}
+
 /**
  * Refuse a response oc cannot read as text. Both transports call this: the
  * gate has to live on whichever client got the page, or the same URL renders
@@ -310,7 +333,18 @@ function wrapNodeResponse(res, url) {
   };
   const text = () => new Promise((resolve, reject) => {
     const chunks = [];
-    res.on('data', (c) => chunks.push(c));
+    let size = 0;
+    res.on('data', (c) => {
+      size += c.length;
+      try {
+        assertBodySize(size, url);
+      } catch (err) {
+        // destroy surfaces the refusal through 'error', and stops the read.
+        res.destroy(err);
+        return;
+      }
+      chunks.push(c);
+    });
     res.on('end', () => resolve(Buffer.concat(chunks).toString('utf8')));
     res.on('error', reject);
   });
@@ -507,7 +541,12 @@ async function viaImpers(impers, target) {
   }
   if (status >= 400) throw new Error(`fetch failed: ${status} for ${target}`);
   assertReadableType(res.headers.get('content-type'));
+  assertBodySize(Number(res.headers.get('content-length')) || 0, target);
+  // impers buffers inside its own binding, so the size of what it already
+  // holds is all there is to check; the bound still stops an oversized body
+  // from travelling any further.
   const html = typeof res.text === 'function' ? await res.text() : String(res.text ?? res.body ?? '');
+  assertBodySize(html.length, target);
   return { url: res.url ?? target, html, status, via };
 }
 
@@ -522,5 +561,27 @@ async function viaFetch(target) {
     throw new Error(`fetch failed: ${res.status} ${res.statusText} for ${current}`);
   }
   assertReadableType(res.headers.get('content-type'));
-  return { url: res.url || current, html: await res.text(), status: res.status, via: 'fetch' };
+  assertBodySize(Number(res.headers.get('content-length')) || 0, current);
+  return { url: res.url || current, html: await readBody(res, current), status: res.status, via: 'fetch' };
+}
+
+/**
+ * The decoded body as text, counted as it arrives so crossing the cap aborts
+ * the transfer instead of finishing it. Throwing mid-iteration cancels the
+ * stream. A proxy response has no web stream to iterate; its text() counts
+ * inside wrapNodeResponse instead.
+ * @param {any} res
+ * @param {string} url
+ * @returns {Promise<string>}
+ */
+export async function readBody(res, url) {
+  if (!res.body?.getReader) return res.text();
+  const chunks = [];
+  let size = 0;
+  for await (const chunk of res.body) {
+    size += chunk.byteLength;
+    assertBodySize(size, url);
+    chunks.push(chunk);
+  }
+  return Buffer.concat(chunks).toString('utf8');
 }

src/render.js

@@ -35,8 +35,9 @@ const num = (v) => v.toLocaleString('en-US');
 // which is what tells a link-list page (Hacker News, search results) from a
 // page whose only links are its own menu.
 const CONTENT_LABEL = 25;
-// Below this there is nothing to read whatever the page is, so how much markup
-// it arrived in does not matter.
+// Below this a render is suspiciously thin, but thin is only a verdict when
+// the page's own size says there should have been more. A terse page that
+// arrived terse (a status endpoint, a one-line answer) distilled fine.
 export const MIN_CONTENT = 25;
 // Below this, with markup that large behind it, the fetch worked and the render
 // did not: a real page of that weight always distills to more. A genuinely
@@ -65,7 +66,11 @@ export const contentTokens = (page) =>
  * @returns {string|null}
  */
 export function contentFailure(content, htmlTokens) {
-  if (content < MIN_CONTENT) return `~${content} tokens of text on the whole page`;
+  // Nothing extracted is empty whatever the page weighed. Anything more is
+  // only a failure with evidence: a small page that renders small is not
+  // gated, it is small, and exit 2 on it would send an agent to a browser
+  // for a page it was already holding.
+  if (content === 0) return 'no text on the whole page';
   if (content < THIN_CONTENT && htmlTokens > THIN_HTML) {
     return `~${content} tokens of text out of ~${htmlTokens} of HTML`;
   }
@@ -92,7 +97,7 @@ export const FINISH = 4;
  */
 export function render(page, { budget = 500, from = 0 } = {}) {
   const blocks = collapseRuns(page.blocks);
-  const head = page.title ? [from > 0 ? `# ${page.title} (continued)` : `# ${page.title}`] : [];
+  const head = page.title ? [from > 0 ? `# ${truncate(page.title)} (continued)` : `# ${truncate(page.title)}`] : [];
   const lines = [...head];
   let spent = estimateTokens(lines.join('\n'));
   let hasLinks = false;
@@ -204,13 +209,13 @@ export function formatBlock(b, { full = false } = {}) {
   const tag = b.n == null ? '' : `[${b.n}] `;
   switch (b.type) {
     case 'heading':
-      return `${'#'.repeat(Math.min(b.level ?? 2, 3))} ${tag}${b.text}`;
+      return `${'#'.repeat(Math.min(b.level ?? 2, 3))} ${tag}${full ? b.text : truncate(b.text)}`;
     case 'link':
       return `${tag}${full ? b.text : truncate(b.text)}`;
     case 'button':
       return `${tag}button "${full ? b.text : truncate(b.text)}"`;
     case 'input':
-      return `${tag}input ${b.name} (${b.text})`;
+      return `${tag}input ${truncate(b.name ?? '')} (${truncate(b.text ?? '')})`;
     case 'divider':
       return b.text;
     default:

tests/act.test.js

@@ -21,6 +21,19 @@ const open = (name = 'default', budget = 500) => {
   saveSession(name, sessionFromPage(p, loadSession(name), { cursor: render(p, { budget }).stats.next }));
 };
 
+test("read cuts even a first block bigger than its whole budget", () => {
+  // The first line of a read always prints, but its text is the page's to
+  // write, so alone-over-budget still cuts: 'up to N tokens' is a promise the
+  // page must not be able to break.
+  const wall = 'sentence after sentence of the same thing. '.repeat(500);
+  const p = distill(`<html><body><p id="wall">${wall}</p></body></html>`, 'https://example.test/wall');
+  saveSession('wall', sessionFromPage(p, null, { cursor: null }));
+  const n = p.blocks.find((b) => b.type === 'text').n;
+  const out = read(n, { session: 'wall', budget: 100 });
+  assert.ok(out.length < 100 * 4 + 200, `read printed ${out.length} chars against a budget of 100 tokens`);
+  assert.match(out, /cut at ~100 tokens, raise --budget/);
+});
+
 test('a rendered page is remembered with absolute URLs for every handle', () => {
   open();
   const state = loadSession('default');

tests/distill.test.js

@@ -490,13 +490,13 @@ test('a page that arrives with no readable text is reported as a failure', () =>
   };
 
   // Nothing at all, whatever the page weighed.
-  assert.match(verdict('<div id="root"></div>', 0), /~0 tokens of text on the whole page/);
+  assert.match(verdict('<div id="root"></div>', 0), /no text on the whole page/);
 
   // Menu links only: short labels are furniture, so this page has no content
   // either, however much markup came with it.
   const chrome = ['Help', 'Log in', 'Content Policy', 'About', 'Careers', 'Press']
     .map((t) => `<a href="/${t}">${t}</a>`).join('');
-  assert.match(verdict(chrome, 60_000), /~0 tokens of text on the whole page/);
+  assert.match(verdict(chrome, 60_000), /no text on the whole page/);
 
   // A consent wall or a login gate: a sentence or two of real text, out of
   // markup far too big to have carried only that.
@@ -508,6 +508,36 @@ test('a page that arrives with no readable text is reported as a failure', () =>
   assert.equal(verdict(gate, 0), null);
 });
 
+test('a terse page that arrived terse is content, not a failed render', () => {
+  // A status endpoint or a one-line answer distills fine and has to exit 0:
+  // calling it gated would send an agent to a browser for a page it was
+  // already holding. Only weight it never rendered is evidence of a gate.
+  const html = '<html><head><title>status</title></head><body><p>All systems operational.</p></body></html>';
+  const page = distill(html, 'https://fixture.test/status');
+  assert.equal(contentFailure(contentTokens(page), estimateTokens(html)), null);
+  const json = distill('{"status":"ok"}', 'https://fixture.test/health');
+  assert.equal(contentFailure(contentTokens(json), 4), null);
+});
+
+test('page-written scalars are capped at the render boundary', () => {
+  // The title and every heading are the page's to write, so without a cap one
+  // hostile scalar prints unbounded output whatever the budget says.
+  const bigTitle = 'title word '.repeat(1000).trim();
+  const bigHeading = 'heading word '.repeat(1000).trim();
+  const page = distill(
+    `<html><head><title>${bigTitle}</title></head><body><h1>${bigHeading}</h1><p>short</p></body></html>`,
+    'https://fixture.test/big');
+  const { text } = render(page, { budget: 100 });
+  for (const line of text.split('\n')) {
+    assert.ok(line.length < 300, `a render line ran to ${line.length} chars`);
+  }
+  assert.match(text, /\.\.\. \+[\d,]+ chars/);
+  // The distilled page keeps the full values: --json is the machine-stable
+  // view, its size is bounded by the fetch cap, and machines cut for
+  // themselves.
+  assert.equal(page.title, bigTitle);
+});
+
 test('a link-list page counts as content even with no prose on it', () => {
   // Hacker News and search results are links and nothing else, so a rule that
   // counted only prose would call the tool's best pages empty.

tests/fetch.test.js

@@ -530,3 +530,41 @@ test('proxyGet refuses a non-HTTP proxy scheme', () => {
     /unsupported proxy protocol \(socks5\)/,
   );
 });
+
+test('a body over the cap is refused, from the header or from the bytes', async () => {
+  const { assertBodySize, readBody, MAX_BODY } = await import('../src/fetch.js');
+
+  // The header check catches a response honest about its size early.
+  assert.doesNotThrow(() => assertBodySize(MAX_BODY, 'https://example.test/big'));
+  assert.throws(() => assertBodySize(MAX_BODY + 1, 'https://example.test/big'), /more than oc will read/);
+
+  // The header is optional and untrusted, so the stream is counted too: a
+  // chunked response crossing the cap fails deterministically, and one just
+  // below it arrives whole.
+  const mb = new Uint8Array(1024 * 1024).fill(120);
+  const stream = (chunks) => new Response(new ReadableStream({
+    start(c) {
+      for (let i = 0; i < chunks; i++) c.enqueue(mb);
+      c.close();
+    },
+  }));
+  await assert.rejects(() => readBody(stream(26), 'https://example.test/bomb'), /more than oc will read/);
+  const small = await readBody(stream(2), 'https://example.test/fine');
+  assert.equal(small.length, 2 * 1024 * 1024);
+});
+
+test('the proxy transport counts the body against the same cap', async () => {
+  const proxy = http.createServer((req, res) => {
+    res.writeHead(200, { 'content-type': 'text/html' });
+    const mb = Buffer.alloc(1024 * 1024, 'x');
+    for (let i = 0; i < 26; i++) res.write(mb);
+    res.end();
+  });
+  const port = await listen(proxy);
+  try {
+    const res = await proxyGet('http://example.test/bomb', `http://127.0.0.1:${port}`);
+    await assert.rejects(() => res.text(), /more than oc will read/);
+  } finally {
+    proxy.close();
+  }
+});