Address review: pager owns limit/page, reject GET+input, label delete exclusivity

README.md

@@ -605,7 +605,7 @@ Calls `POST /api/v1/repos/{owner}/{repo}/labels`.
 
 ### `stoke label delete`
 
-Delete a label from a repository, by `--id` or `--name` (one is required).
+Delete a label from a repository, by `--id` or `--name` (exactly one is required; passing both is rejected).
 
 ```bash
 stoke label delete -o heavy-duty -r stoke --name needs-triage
@@ -853,7 +853,8 @@ Options:
   -X, --method <method>    GET, POST, PUT, PATCH or DELETE
                            (default: GET, or POST when --input is given)
   --input <json>           JSON request body, inline or @path to read from a file
-  --paginate               fetch all pages (GET endpoints returning a JSON array)
+  --paginate               fetch all pages (GET endpoints returning a JSON array);
+                           overrides any limit/page in the endpoint
 ```
 
 ```bash
@@ -863,7 +864,9 @@ stoke api /repos/heavy-duty/stoke/issues/12/comments --input '{"body":"hi"}'
 stoke api /repos/heavy-duty/stoke/contents/CHANGELOG.md --input @payload.json
 ```
 
-Calls `{METHOD} /api/v1{endpoint}` with the stored token. The endpoint must start with `/`; the method, `--paginate` + non-GET, and malformed `--input` JSON are all rejected before any network call.
+Calls `{METHOD} /api/v1{endpoint}` with the stored token. The endpoint must start with `/`; the method, `--paginate` + non-GET, `GET` + `--input` (a GET cannot carry a body), and malformed `--input` JSON are all rejected before any network call.
+
+**Security:** `stoke api` is a full authenticated passthrough — it does anything the stored token is allowed to do. Never interpolate untrusted strings (issue titles, PR bodies, user input) into the endpoint or `--input`; treat every call like the credential it carries.
 
 ## Architecture
 

src/api.js

@@ -134,10 +134,20 @@ class ForgejoClient {
   async getAll(endpoint, params = {}) {
     const pageSize = 50;
     const all = [];
-    const separator = endpoint.includes('?') ? '&' : '?';
+    // The pager owns limit/page: a caller-supplied pair must be overridden,
+    // not duplicated — a duplicated limit pins the page size the server
+    // honors first and can truncate or loop the walk.
+    const queryIndex = endpoint.indexOf('?');
+    const path = queryIndex === -1 ? endpoint : endpoint.slice(0, queryIndex);
+    const baseQuery = new URLSearchParams(queryIndex === -1 ? '' : endpoint.slice(queryIndex + 1));
+    baseQuery.delete('limit');
+    baseQuery.delete('page');
     for (let page = 1; page <= 1000; page += 1) {
-      const query = new URLSearchParams({ ...params, limit: String(pageSize), page: String(page) }).toString();
-      const items = await this.get(`${endpoint}${separator}${query}`);
+      const query = new URLSearchParams(baseQuery);
+      for (const [key, value] of Object.entries(params)) query.set(key, value);
+      query.set('limit', String(pageSize));
+      query.set('page', String(page));
+      const items = await this.get(`${path}?${query.toString()}`);
       if (!Array.isArray(items) || items.length === 0) break;
       all.push(...items);
       if (items.length < pageSize) break;

src/cli.js

@@ -973,6 +973,10 @@ label
         console.error('One of --id or --name is required.');
         process.exit(1);
       }
+      if (options.id && options.name) {
+        console.error('Use either --id or --name, not both.');
+        process.exit(1);
+      }
       const config = loadConfig();
       const client = ForgejoClient.fromConfig(config);
       const ids = options.id
@@ -1365,6 +1369,10 @@ program
         console.error('--paginate only works with GET.');
         process.exit(1);
       }
+      if (method === 'GET' && options.input !== undefined) {
+        console.error('GET requests cannot carry a body. Drop --input, or use -X POST/PUT/PATCH/DELETE.');
+        process.exit(1);
+      }
 
       let body = null;
       if (options.input !== undefined) {

test/api.test.js

@@ -187,6 +187,22 @@ test('getAll joins pagination with & when the endpoint already has a query', asy
   assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/o/r/pulls?state=closed&limit=50&page=1');
 });
 
+test('getAll overrides caller-supplied limit/page instead of duplicating them', async () => {
+  const calls = mockFetch((url) => {
+    const page = Number(new URL(url).searchParams.get('page'));
+    return jsonResponse(page === 1 ? Array.from({ length: 50 }, (_, i) => ({ id: i })) : []);
+  });
+  const client = new ForgejoClient('https://forge.test', 'tok');
+  const all = await client.getAll('/repos/o/r/pulls?state=closed&limit=1&page=9');
+  assert.equal(all.length, 50);
+  const first = new URL(calls[0].url).searchParams;
+  const second = new URL(calls[1].url).searchParams;
+  assert.deepEqual(first.getAll('limit'), ['50']);
+  assert.deepEqual(first.getAll('page'), ['1']);
+  assert.deepEqual(second.getAll('page'), ['2']);
+  assert.equal(first.get('state'), 'closed');
+});
+
 test('release endpoints map to the expected URLs and payloads', async () => {
   const calls = mockFetch(() => jsonResponse({ tag_name: '1.0.0' }));
   const client = new ForgejoClient('https://forge.test', 'tok');
@@ -203,13 +219,15 @@ test('release endpoints map to the expected URLs and payloads', async () => {
 test('label endpoints map to the expected URLs and payloads', async () => {
   const calls = mockFetch(() => jsonResponse({ id: 3 }));
   const client = new ForgejoClient('https://forge.test', 'tok');
+  await client.listLabels('owner', 'repo');
   await client.createLabel('owner', 'repo', { name: 'release', color: '0e8a16', description: '' });
   await client.deleteLabel('owner', 'repo', 3);
-  assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/labels');
-  assert.equal(calls[0].opts.method, 'POST');
-  assert.deepEqual(JSON.parse(calls[0].opts.body), { name: 'release', color: '0e8a16', description: '' });
-  assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/labels/3');
-  assert.equal(calls[1].opts.method, 'DELETE');
+  assert.equal(calls[0].url, 'https://forge.test/api/v1/repos/owner/repo/labels?limit=50&page=1');
+  assert.equal(calls[1].url, 'https://forge.test/api/v1/repos/owner/repo/labels');
+  assert.equal(calls[1].opts.method, 'POST');
+  assert.deepEqual(JSON.parse(calls[1].opts.body), { name: 'release', color: '0e8a16', description: '' });
+  assert.equal(calls[2].url, 'https://forge.test/api/v1/repos/owner/repo/labels/3');
+  assert.equal(calls[2].opts.method, 'DELETE');
 });
 
 test('issue label add/remove hit the issue labels endpoints', async () => {

test/cli.test.js

@@ -206,6 +206,18 @@ test('api rejects --paginate with a non-GET method before any network call', ()
   assert.match(res.stderr, /--paginate only works with GET/);
 });
 
+test('api rejects GET with --input before any network call', () => {
+  const res = run(['api', '/user', '-X', 'GET', '--input', '{}']);
+  assert.equal(res.status, 1);
+  assert.match(res.stderr, /GET requests cannot carry a body/);
+});
+
+test('label delete rejects --id and --name together before any network call', () => {
+  const res = run(['label', 'delete', '-o', 'o', '-r', 'r', '--id', '3', '--name', 'x']);
+  assert.equal(res.status, 1);
+  assert.match(res.stderr, /either --id or --name, not both/);
+});
+
 test('api rejects invalid JSON input before any network call', () => {
   const res = run(['api', '/user', '--input', '{nope']);
   assert.equal(res.status, 1);
@@ -251,6 +263,45 @@ test('api sends the token and prints the JSON response', async () => {
   }
 });
 
+test('label add fails closed on an unknown label name', async () => {
+  const cfg = path.join(os.tmpdir(), `stoke-cfg-${process.pid}-lbl.json`);
+  const TIMEOUT_MS = 5000;
+  let timer;
+  const result = await new Promise((resolve, reject) => {
+    const fail = (err) => {
+      clearTimeout(timer);
+      try { server.close(); } catch { /* already closed */ }
+      reject(err instanceof Error ? err : new Error(String(err)));
+    };
+    const server = http.createServer((req, res) => {
+      res.writeHead(200, { 'Content-Type': 'application/json' });
+      res.end(JSON.stringify([{ id: 1, name: 'bug', color: 'd73a4a' }]));
+    });
+    timer = setTimeout(() => fail(new Error('timeout')), TIMEOUT_MS);
+    server.listen(0, '127.0.0.1', async () => {
+      const { port } = server.address();
+      fs.writeFileSync(cfg, JSON.stringify({ url: `http://127.0.0.1:${port}`, token: 'tok' }));
+      try {
+        const res = await spawnAsync(
+          ['label', 'add', '-o', 'o', '-r', 'r', '-n', '7', '--name', 'ghost'],
+          { STOKE_CONFIG_FILE: cfg },
+        );
+        clearTimeout(timer);
+        server.close(() => resolve(res));
+      } catch (err) {
+        fail(err);
+      }
+    });
+  }).finally(() => clearTimeout(timer));
+
+  try {
+    assert.equal(result.status, 1);
+    assert.match(result.stderr, /Label not found in o\/r: ghost/);
+  } finally {
+    fs.unlinkSync(cfg);
+  }
+});
+
 function spawnAsync(args, env = {}) {
   return new Promise((resolve, reject) => {
     const child = spawn(process.execPath, [CLI, ...args], {