Skip to content

fix: stop uploading a duplicate SSH key on key-heavy accounts#9

Merged
janakkapadia merged 1 commit into
mainfrom
fix/ssh-key-duplicate-upload
Jul 13, 2026
Merged

fix: stop uploading a duplicate SSH key on key-heavy accounts#9
janakkapadia merged 1 commit into
mainfrom
fix/ssh-key-duplicate-upload

Conversation

@vikasiwp

@vikasiwp vikasiwp commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Root cause

ensureSshAccess() works out whether its key is already on the account by listing GET /ssh-keys — but that endpoint is paginated (DEFAULT_TEMPLATE_PAGINATION = 10/page, newest first) and the CLI read only page 1, ignoring meta.last_page.

Once an account's key list spills past page 1, the CLI's own key is invisible to it → it concludes "not uploaded" → uploads another copy of a key it already has (POST /ssh-keys, label InstaWP CLI) and attaches that to the site instead of the original.

How it accumulates (verified in QA — and it's slower than "one per site"): the fresh duplicate is now the newest key, so it sits on page 1 and the next site finds it. The count doesn't grow again until ≥10 newer keys push it back off page 1. So it's one duplicate per "key fell off page 1" episode, not one per site. User-visible symptom is the same: identical InstaWP CLI keys accumulating, and the site attached to a duplicate rather than the key the user already had.

(POST /sites/{id}/ssh-keys/{keyId} is already idempotent, so the duplication is purely in the account key list.)

Fix

fetchUploadedKeys() now pages through the whole list (per_page=100, follows meta.last_page, hard-capped at 20 pages) so matching sees every key on the account. No behaviour change when the account has a single page of keys.

Test

New regression test: the CLI key sits on page 2 of the account list → it must be matched and attached (id 7), with no upload. It fails on main and passes here. Full suite: 486 passing, typecheck clean.

QA: PASS ✅

Verified end-to-end against the production API (which does not carry the server-side fix, so this isolates PR #9's effect). The pre-fix build reproduced the duplicate; the PR build produced zero uploads across two fresh sites with the CLI key on page 2, reusing the existing key. First-ever-run path still creates exactly one key. Full evidence in the QA comment below.

Companion PR

InstaWP/client-app#2609 makes POST /ssh-keys idempotent on identical key material. That closes the fail-open path this PR can't: a failed list call is silently treated as "the account has no keys", and concurrent instawp runs can race. Also QA-passed. The pair should land together.

Run /code-review ultra <PR#> before merge.

— Engineer (agent-os)

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 <noreply@anthropic.com>
@vikasiwp

Copy link
Copy Markdown
Contributor Author

QA: PASS ✅

Env: throwaway InstaWP sandboxes on a test account (our own; 5 pre-existing keys) · InstaWP/cli PR #9 @157d7b6 · API = production app.instawp.io, which does not have the server-side fix — so this isolates PR #9's effect on its own.
Criteria: on a key-heavy account (CLI key off page 1 of GET /ssh-keys), running an SSH-path command against ≥2 sites must not grow the account's key list; the existing key must be the one attached; SSH/WP-CLI must still work. First-ever run on a low-key account must still upload exactly one key.

1. Repro before the fix (pre-fix build @7a7b6fc) — confirmed ❌

Seeded 10 keys so the CLI's own key (id 974, same material as ~/.instawp/cli_key.pub) sat on page 2:

GET /ssh-keys  → meta {current_page: 1, last_page: 2, total_count: 15}
page 1: 1016 1015 1014 1013 1012 1011 1010 1009 1008 1007   ← all seeds
page 2: 1005, 974 (InstaWP CLI), 962, 945, 697              ← the CLI's key, invisible to the CLI

instawp wp qa-sshdup-a -- option get siteurl

TOTAL KEYS: 15 → 16
new key 1017 "InstaWP CLI", created 2026-07-13T10:18:26Z
material of 1017 == material of 974  ← byte-identical duplicate of a key it already had
site 2565822 attached key: 1017 (the duplicate, not the original)

Nuance worth recording: the duplicate is uploaded once per "key is off page 1" episode, not literally once per site. After the first upload, the new copy is the newest key, so it lands on page 1 and the very next site finds it (site B reused 1017; count stayed 16). The list only grows again once ≥10 newer keys push it off page 1 again. The pile-up is real but slower than "one per site" — the PR description may want a tweak.

2. After the fix (PR #9) — no duplicates ✅

Restored the exact precondition (15 keys, exactly one InstaWP CLI key = 974, on page 2), created two fresh sites, ran the PR build against both:

instawp wp qa-sshfix-c -- option get siteurl → ✓ SSH access ready → https://qa-sshfix-c.instawp.site
instawp wp qa-sshfix-d -- option get siteurl → ✓ SSH access ready → https://qa-sshfix-d.instawp.site
instawp wp qa-sshfix-d -- core version       → 7.0

TOTAL KEYS after both sites: 15   (unchanged — no upload)
keys with the CLI key material: [974]           ← exactly one, the pre-existing June key
InstaWP CLI keys created during the run: []     ← none
site 2565866 (C) attached key: 974  ✅
site 2565868 (D) attached key: 974  ✅

Paged matching works: the key on page 2 is found and reused, and it's the one attached to each site. SSH + WP-CLI work end-to-end.

3. First-ever run still works (low-key account) ✅

Virgin CLI home (no ~/.instawp/cli_key), account back to its 5 keys:

→ CLI generated a new local key, uploaded it: exactly ONE new row (id 1018). TOTAL 5 → 6
→ wp qa-sshfix-c -- option get siteurl  → ✓ works
→ second site with the same key         → TOTAL still 6 (no re-upload)

4. Unit tests

npx vitest run50 files / 486 tests passed, including the new "matches a key that is not on the first page" regression test (17/17 in ssh-keys.test.ts).

Teardown: all 4 sandbox sites deleted, all 10 seeded keys + both test-created keys removed. Account verified byte-for-byte back to its baseline 5 keys.

Note for the reviewer: this PR is one half of the pair — it stops the CLI initiating the duplicate. The fail-open path it can't close (list call errors → CLI treats it as "no keys" → uploads) is covered by InstaWP/client-app#2609, which I verified separately (PASS): with that patch, even a client that re-uploads the same key gets the existing row back, so no duplicate row is created.

— QA

@janakkapadia janakkapadia merged commit 15e7782 into main Jul 13, 2026
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants