[nwparker]

ELI5

Your computer keeps an address book of servers it trusts, called known_hosts. Each line has a scrambled server name and a fingerprint, both written in a code called base64.

We were reading that code too forgivingly. If a line had junk characters smudged into it, the tool we used to decode it would quietly skip the junk and hand back a perfectly good-looking answer — so we'd say "yes, I recognise this server." But real ssh throws that whole line in the bin as unreadable. So we could end up trusting a line your own ssh refuses to even look at.

This makes us check that the code is written exactly right, by decoding it and then re-encoding it to see if we get back what we started with. There was already a check like that, but only on one of the fields — the other two didn't have it. Now all three go through the same one check, so the next person to add a field can't forget it.

What you might notice: nothing, unless your known_hosts has a hand-edited or corrupted line. If it does, that line now gets ignored — the same thing ssh already does with it.

---

	Files	Added	Deleted	Net
Test	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​46	0	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​46
Prod	1	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​27	$\color{#cf222e}{\Huge{\mathbf{−}}}$​15	$\color{#1a7f37}{\Huge{\mathbf{+}}}$​12

Resolves STA-4717. Follow-up from #14844.

Problem

parseHashedPatterns decoded the salt and hash fields with plain Buffer.from(x, 'base64') plus a 20-byte length check, while decodeKey — a few lines above, in the same file — additionally re-encoded and compared to reject non-canonical input.

That gap matters because Buffer.from(x, 'base64') never throws: it silently skips invalid characters. A line real ssh rejects outright (parse error in hostkeys file) could therefore parse here and be trusted by us. The length check isn't sufficient on its own — garbage characters get skipped and the result can still be exactly 20 bytes.

Root cause

The strictness rule was implemented ad hoc at one call site rather than being the decode primitive every base64 field in the parser must go through. Copying the re-encode check into parseHashedPatterns would have fixed this instance and left the next field free to forget it again.

Fix

One strict decoder, used by both the key field and the hashed salt/hash fields, so a future field cannot bypass it.

Tests

Added cases to ssh-known-hosts.test.ts covering non-canonical salt and hash fields that decode to the correct length but are not what ssh accepts. 78 tests pass.

[coderabbitai]

📝 Walkthrough

Walkthrough

Known-hosts parsing now uses a shared canonical Base64 decoder for keys, hashed salts, and hashes. The decoder rejects empty, malformed, non-canonical, URL-safe, unpadded, and over-padded values. Hashed salts and hashes must decode to 20 bytes. Tests cover malformed fields and confirm that malformed entries return unknown, while the canonical entry returns match.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely summarizes the main change: strict Base64 validation for hashed known_hosts fields.
Description check	✅ Passed	The description explains the problem, root cause, fix, linked issue, behavior impact, and tests, but it omits several template headings and checklist details.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

src/main/ssh/ssh-known-hosts.ts (1)

89-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reduce the Base64 rationale comments.

Keep only the non-obvious OpenSSH compatibility rule. Remove detailed decoder behavior and manual verification narratives.

• src/main/ssh/ssh-known-hosts.ts#L89-L104: reduce the decoder documentation to a brief canonical-Base64 requirement.
• src/main/ssh/ssh-known-hosts.ts#L122-L125: reduce the length-check rationale to one short requirement statement.
• src/main/ssh/ssh-known-hosts.test.ts#L356-L360: rely on the parameterized test names instead of documenting the full reproduction details.
• src/main/ssh/ssh-known-hosts.test.ts#L364-L367: reduce the padding-bit explanation to a brief reason for the fixture.

As per coding guidelines, comments must be concise, non-obvious, and brief.

Source: Coding guidelines

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ce256719-84d0-4864-8410-3c673014a803

📥 Commits

Reviewing files that changed from the base of the PR and between 2a760e3 and 77632f0.

📒 Files selected for processing (2)

• src/main/ssh/ssh-known-hosts.test.ts
• src/main/ssh/ssh-known-hosts.ts

Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review.

[nwparker]

Cross-platform verification

Run on real hosts via orca-cli, not emulated.

Linux — Ubuntu 24.04 (kernel 7.0.0), node v26.1.0, pnpm 10.24.0, over SSH.
Windows — awin, Git Bash (MINGW64), node v24.18.0, WSL Ubuntu-24.04 + Sta4593-Federated both running.

host	result
Linux	Test Files 1 passed / Tests 78 passed
Windows	Test Files 1 passed

Clean on both.