From 157d7b624d7c2314ed5b26e16353fa452da1561b Mon Sep 17 00:00:00 2001 From: "InstaWP Engineer (bot)" Date: Mon, 13 Jul 2026 09:59:51 +0000 Subject: [PATCH] fix: stop uploading a duplicate SSH key on key-heavy accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ensureSshAccess() decides whether its key is already on the account by listing GET /ssh-keys — but that endpoint is paginated (10/page, newest first) and the CLI read only page 1 and ignored meta.last_page. On an account with more keys than fit on page 1, the CLI's own key is invisible, so it concludes 'not uploaded' and uploads ANOTHER copy, then attaches it to the site. The SSH connection cache is per-site, so this repeats for every site the user touches and duplicate 'InstaWP CLI' keys pile up in their account. Fetch every page (per_page=100, follow meta.last_page, hard-capped at 20 pages) so matching sees the whole key list. Co-Authored-By: Claude Opus 4.8 --- src/__tests__/ssh-keys.test.ts | 38 ++++++++++++++++++++++++ src/lib/ssh-keys.ts | 53 ++++++++++++++++++++++++++-------- 2 files changed, 79 insertions(+), 12 deletions(-) diff --git a/src/__tests__/ssh-keys.test.ts b/src/__tests__/ssh-keys.test.ts index 2b9fa70..dbb2a94 100644 --- a/src/__tests__/ssh-keys.test.ts +++ b/src/__tests__/ssh-keys.test.ts @@ -176,6 +176,44 @@ describe('ssh-keys', () => { expect(uploadCalls.length).toBe(0); }); + it('matches a key that is not on the first page of the account key list', async () => { + // Regression: the key list is paginated (10/page by default, newest first). Reading only + // page 1 made an already-uploaded key look missing on key-heavy accounts, so the CLI + // uploaded a duplicate of its own key — once per site. + mockFiles[CLI_KEY_PUB] = 'ssh-rsa AAAACLIKEY== instawp-cli'; + mockFiles[CLI_KEY_PATH] = 'private key'; + + // API: ssh-keys page 1 — someone else's newer keys, ours isn't here + mockGet.mockResolvedValueOnce({ + data: { + data: [{ id: 1, label: 'Laptop', ssh_key: 'ssh-rsa AAAAOTHER== laptop' }], + meta: { current_page: 1, last_page: 2 }, + }, + }); + // API: ssh-keys page 2 — the CLI key we uploaded ages ago + mockGet.mockResolvedValueOnce({ + data: { + data: [{ id: 7, label: 'InstaWP CLI', ssh_key: 'ssh-rsa AAAACLIKEY== instawp-cli' }], + meta: { current_page: 2, last_page: 2 }, + }, + }); + // API: enable SSH + mockPost.mockResolvedValueOnce({ data: { host: 'paged.com', username: 'paged', port: 22, data: [] } }); + // API: enable SFTP + mockPost.mockResolvedValueOnce({ data: {} }); + // API: attach key + mockPost.mockResolvedValueOnce({ data: {} }); + // API: site details + mockGet.mockResolvedValueOnce({ data: { data: { site: { main_domain: 'paged.com' } } } }); + + const result = await ensureSshAccess(250); + expect(result.host).toBe('paged.com'); + // No duplicate upload... + expect(mockPost.mock.calls.filter((c: any[]) => c[0] === '/ssh-keys').length).toBe(0); + // ...and the existing key (id 7, from page 2) is the one attached to the site + expect(mockPost.mock.calls.some((c: any[]) => c[0] === '/sites/250/ssh-keys/7')).toBe(true); + }); + it('exits on 403 when SSH requires paid plan', async () => { mockFiles[CLI_KEY_PUB] = 'ssh-rsa AAAA== instawp-cli'; mockFiles[CLI_KEY_PATH] = 'private key'; diff --git a/src/lib/ssh-keys.ts b/src/lib/ssh-keys.ts index ac3f159..420f3a9 100644 --- a/src/lib/ssh-keys.ts +++ b/src/lib/ssh-keys.ts @@ -13,6 +13,9 @@ const INSTAWP_DIR = path.join(homedir(), '.instawp'); const CLI_KEY_PATH = path.join(INSTAWP_DIR, 'cli_key'); const CLI_KEY_PUB_PATH = CLI_KEY_PATH + '.pub'; +const KEY_PAGE_SIZE = 100; // per_page for GET /ssh-keys (API default is 10) +const MAX_KEY_PAGES = 20; // hard stop, so a bad `meta` can't spin forever + function ensureInstawpDir(): void { if (!existsSync(INSTAWP_DIR)) { mkdirSync(INSTAWP_DIR, { recursive: true }); @@ -73,6 +76,42 @@ function generateCliKey(): { privatePath: string; pubContent: string } { }; } +/** + * Every SSH key on the account, across all pages. + * + * `GET /ssh-keys` is paginated (10/page by default, newest first) and we only ever read + * page 1 — so on an account with more keys than that, an already-uploaded CLI key is + * invisible, we conclude "not uploaded", and upload ANOTHER copy of it. Since the SSH + * connection cache is per-site, that repeats for every site the user touches: the "InstaWP + * CLI" key piles up in their account. Ask for a big page and follow `meta.last_page`, so + * matching sees the whole list. + */ +async function fetchUploadedKeys(): Promise { + const client = getClient(); + const keys: SshKeyInfo[] = []; + + try { + for (let page = 1; page <= MAX_KEY_PAGES; page++) { + const res = await client.get('/ssh-keys', { params: { per_page: KEY_PAGE_SIZE, page } }); + const items = res.data?.data; + if (!Array.isArray(items)) break; + + for (const k of items) { + keys.push({ id: k.id, label: k.label || '', ssh_key: k.ssh_key || '' }); + } + + const lastPage = Number(res.data?.meta?.last_page); + if (!Number.isFinite(lastPage) || page >= lastPage) break; + } + } catch { + // SSH keys endpoint might not exist / transient failure — fall through with whatever + // we got. An empty list means step 5 uploads; the API is idempotent on identical key + // material, so that can't duplicate an existing key. + } + + return keys; +} + export interface EnsureSshOptions { /** Override the SSH host (from --ssh-host or INSTAWP_SSH_HOST*); beats the API host. */ sshHost?: string; @@ -148,18 +187,8 @@ async function resolveConnection(siteId: number, override: string | null): Promi // 2. Find local keys let localKeys = findLocalPubKeys(); - // 3. Fetch uploaded keys from API - let uploadedKeys: SshKeyInfo[] = []; - try { - const res = await client.get('/ssh-keys'); - uploadedKeys = (res.data?.data || []).map((k: any) => ({ - id: k.id, - label: k.label || '', - ssh_key: k.ssh_key || '', - })); - } catch { - // SSH keys endpoint might not exist; proceed to upload - } + // 3. Fetch uploaded keys from API (every page — see fetchUploadedKeys) + const uploadedKeys = await fetchUploadedKeys(); // 4. Find a matching key (local key already uploaded) let matchedKey: { privatePath: string; keyId: number } | null = null;