fix(ssh): reject hostnames with stray leading or trailing colons in parseSshTarget by miorbnli · Pull Request #93887 · openclaw/openclaw

miorbnli · 2026-06-17T03:09:54Z

Summary

parseSshTarget currently returns a host field that still contains a stray colon when the input has a trailing colon or a leading colon (e.g. "host:", ":22", "user@host:"). The value flows directly into generated SSH config files (see src/agents/sandbox/ssh.ts:537) and the ssh CLI argv, which causes the resulting connection to fail with an invalid HostName.

This patch adds a guard that rejects any host part that starts or ends with :. Existing valid inputs ("host", "host:22", "user@host:22") keep working.

Linked context

N/A (no existing issue)

Real behavior proof

Behavior or issue addressed: parseSshTarget returned { host: "host:", port: 22 } for input "host:" instead of rejecting it as invalid.
Real environment tested: Node 22.19, Linux x86_64, local checkout of openclaw/openclaw main (bfc5e49), TypeScript source executed directly via tsx.
Exact steps or command run after this patch:
```
node --import tsx /tmp/verify-parseSshTarget.mjs
```
Script imports parseSshTarget from the patched source and runs 7 inputs covering regressions and edge cases.

Evidence after fix: Terminal capture of node runtime verification (copied live output):

=== parseSshTarget runtime verification ===

[PASS] input="host:" (trailing colon without port)
       expected: invalid → null
       got:      null

[PASS] input=":22" (leading colon without host)
       expected: invalid → null
       got:      null

[PASS] input="user@:22" (user with leading-colon host)
       expected: invalid → null
       got:      null

[PASS] input="user@host:" (user@host with trailing colon)
       expected: invalid → null
       got:      null

[PASS] input="host" (plain hostname)
       expected: valid → object
       got:      {"host":"host","port":22}

[PASS] input="host:22" (host:port)
       expected: valid → object
       got:      {"host":"host","port":22}

[PASS] input="me@example.com:2222" (user@host:port)
       expected: valid → object
       got:      {"user":"me","host":"example.com","port":2222}

=== ALL PASSED ===

Observed result after fix: All 4 previously-broken inputs now correctly return null; all 3 previously-valid inputs continue to return correct parsed objects.
What was not tested: End-to-end SSH tunnel creation with these edge-case inputs (covered by the null rejection + existing callers already null-check parseSshTarget result before using).

Tests and validation

$ pnpm test src/infra/ssh-tunnel.test.ts
✓ Test Files  1 passed (1)
✓ Tests       4 passed (4)

$ pnpm build
✓ built successfully

Risk checklist

Does this change affect user-visible behavior? No — invalid inputs that previously produced broken SSH configs are now rejected early with a clear null return.
Does this change affect configuration? No
Does this change affect security? Yes (positive) — reduces risk of malformed host values reaching the SSH CLI argv.
Does this change affect plugins or providers? No
Does this change affect docs? No
Is this change backwards-compatible? Yes — all previously-valid inputs remain valid; only previously-broken inputs are now rejected instead of producing garbage.

…arseSshTarget parseSshTarget would previously return host values like "host:" or ":22" when the input had a trailing colon with no port, or a leading colon with no host. These values flow directly into SSH config HostName fields and the ssh CLI argv, which causes connections to fail. Add a guard that rejects host parts starting or ending with ":" before the existing "-" prefix check. Existing valid inputs ("host", "host:22", "user@host:22") are unaffected. Co-Authored-By: Claude <noreply@anthropic.com>

clawsweeper · 2026-06-17T03:53:13Z

Codex review: needs maintainer review before merge. Reviewed June 20, 2026, 11:07 PM ET / 03:07 UTC.

Summary
The PR adds a shared isMalformedHost guard in parseSshTarget and tests that SSH host parts with stray leading or trailing colons return null.

PR surface: Source +5, Tests +11. Total +16 across 2 files.

Reproducibility: yes. Current main source shows parseSshTarget can leave a leading or trailing colon inside the returned host value, and callers then use that parsed host for SSH config or tunnel startup.

Review metrics: none identified.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🐚 platinum hermit
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

none.

Next step before merge

No ClawSweeper repair lane is needed; this PR is ready for ordinary maintainer review rather than automated code changes.

Security
Cleared: The diff narrows SSH target parsing and adds tests without changing dependencies, workflows, lockfiles, secrets handling, or broader code execution surfaces.

Review details

Best possible solution:

Land the shared parser-boundary validation with its regression coverage, keeping malformed SSH targets rejected before SSH config or argv construction.

Do we have a high-confidence way to reproduce the issue?

Yes. Current main source shows parseSshTarget can leave a leading or trailing colon inside the returned host value, and callers then use that parsed host for SSH config or tunnel startup.

Is this the best way to solve the issue?

Yes. The parser is the shared validation boundary for sandbox SSH config generation, gateway status tunnel startup, and discovery filtering, so one guard there is narrower and cleaner than adding caller-specific rejection.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against fce586538a50.

Label changes

Label justifications:

P2: This is a narrow SSH target parsing bug with limited blast radius but real user-facing connection failures for malformed configured targets.
rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🐚 platinum hermit and patch quality is 🐚 platinum hermit.
status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body includes copied live Node/tsx output from patched source showing malformed SSH targets now return null while valid targets still parse.
proof: sufficient: Contributor real behavior proof is sufficient. The PR body includes copied live Node/tsx output from patched source showing malformed SSH targets now return null while valid targets still parse.

Evidence reviewed

PR surface:

Source +5, Tests +11. Total +16 across 2 files.

View PR surface stats

Area	Files	Added	Removed	Net
Source	1	9	4	+5
Tests	1	11	0	+11
Docs	0	0	0	0
Config	0	0	0	0
Generated	0	0	0	0
Other	0	0	0	0
Total	2	20	4	+16

What I checked:

Current main parser gap: Current main splits on the last colon and rejects empty hosts, invalid ports, and leading dash hosts, but it does not reject host values that still start or end with a colon. (src/infra/ssh-tunnel.ts:39, fce586538a50)
Sandbox caller impact: The SSH sandbox settings path parses settings.target and writes parsed.host directly into the generated HostName field, so malformed parser output reaches SSH config generation. (src/agents/sandbox/ssh.ts:537, fce586538a50)
Gateway caller impact: Gateway status discovery resolves configured SSH targets through the same parser before ssh-config resolution, supporting a shared parser-boundary fix rather than caller-specific filtering. (src/commands/gateway-status/discovery.ts:57, fce586538a50)
Documented target contract: The documented sandbox SSH target shape is user@host[:port], which does not include leading or trailing colon host text. Public docs: docs/gateway/config-agents.md. (docs/gateway/config-agents.md:903, fce586538a50)
Patch behavior: The PR diff adds isMalformedHost and applies it to both explicit-port and default-port parse branches, preserving the existing dash-prefix guard while rejecting leading/trailing colon hosts. (src/infra/ssh-tunnel.ts:24, bafe4dcdd14b)
Regression coverage: The PR adds tests for host:, :22, user@:22, user@host:, host::22, and :host:22, covering both parser branches affected by stray colon host text. (src/infra/ssh-tunnel.test.ts:52, bafe4dcdd14b)

Likely related people:

steipete: Git history shows Peter Steinberger added earlier SSH target hardening, moved SSH sandboxing into core, and centralized gateway discovery target handling around this parser boundary. (role: introduced behavior and adjacent owner; confidence: high; commits: 06289b36da72, b8bb8510a2a3, fe5819887b57; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts, src/agents/sandbox/ssh.ts)
vincentkoc: Current checkout blame for the central SSH parser and sandbox caller resolves through Vincent Koc's recent broad source commit, making this a useful current-source routing signal even though older feature history is stronger. (role: recent area contributor; confidence: medium; commits: 43e8c29fbfdb, fce586538a50; files: src/infra/ssh-tunnel.ts, src/infra/ssh-tunnel.test.ts, src/agents/sandbox/ssh.ts)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

…arseSshTarget (openclaw#93887) * fix(ssh): reject hostnames with stray leading or trailing colons in parseSshTarget parseSshTarget would previously return host values like "host:" or ":22" when the input had a trailing colon with no port, or a leading colon with no host. These values flow directly into SSH config HostName fields and the ssh CLI argv, which causes connections to fail. Add a guard that rejects host parts starting or ending with ":" before the existing "-" prefix check. Existing valid inputs ("host", "host:22", "user@host:22") are unaffected. Co-Authored-By: Claude <noreply@anthropic.com> * fix(ssh): validate stray-colon host in explicit-port parse branch --------- Co-authored-by: Claude <noreply@anthropic.com>

miorbnli closed this Jun 17, 2026

miorbnli reopened this Jun 17, 2026

miorbnli closed this Jun 17, 2026

miorbnli reopened this Jun 17, 2026

miorbnli closed this Jun 17, 2026

miorbnli reopened this Jun 17, 2026

clawsweeper Bot added the P2 Normal backlog priority with limited blast radius. label Jun 19, 2026

fix(ssh): validate stray-colon host in explicit-port parse branch

bafe4dc

vincentkoc merged commit a2b3aab into openclaw:main Jun 22, 2026
184 of 190 checks passed

Navigation Menu

Uh oh!

fix(ssh): reject hostnames with stray leading or trailing colons in parseSshTarget - #93887

fix(ssh): reject hostnames with stray leading or trailing colons in parseSshTarget#93887
vincentkoc merged 2 commits into
openclaw:mainfrom
miorbnli:fix/ssh-parse-target-trailing-colon

miorbnli commented Jun 17, 2026 •

edited

Loading

Uh oh!

clawsweeper Bot commented Jun 17, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

miorbnli commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Linked context

Real behavior proof

Tests and validation

Risk checklist

Uh oh!

clawsweeper Bot commented Jun 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

miorbnli commented Jun 17, 2026 •

edited

Loading

clawsweeper Bot commented Jun 17, 2026 •

edited

Loading