smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(omp): reject ssh:// URL passwords and empty userinfo (#3553 review)

Browse Source 
resolveTarget now rejects a non-empty password (ssh://user:pass@host, ssh://:pw@host) since ssh:// uses key/agent auth (BatchMode), and a literal empty username (ssh://@host) that previously fell through to the bare-host fallback and could match a configured host. Both use the same decoded-authority comparison as the empty-port guard, so a percent-encoded alias like %40prod (decodes to @prod) is still matched.
This commit is contained in:
Tommaso Fontana
2026-06-26 20:41:36 +02:00
parent 36e82776cf
commit da2bccfdf9
2 changed files with 20 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/internal-urls/__tests__/ssh-protocol.test.ts
+8
View File
@@ -260,6 +260,14 @@ describe("SshProtocolHandler", () => {
await expect(handler.resolve(parseInternalUrl("ssh://u%2Dname@prod:/etc/hosts"))).rejects.toThrow(/empty port/);
});
it("rejects ssh:// password and empty-username userinfo before matching a host", async () => {
mockHosts([{ name: "prod", host: "10.0.0.5", _source: SOURCE }]);
await expect(handler.resolve(parseInternalUrl("ssh://user:pass@prod/etc/hosts"))).rejects.toThrow(/password/);
await expect(handler.resolve(parseInternalUrl("ssh://:pw@prod/etc/hosts"))).rejects.toThrow(/password/);
await expect(handler.resolve(parseInternalUrl("ssh://@prod/etc/hosts"))).rejects.toThrow(/empty username/);
await expect(handler.resolve(parseInternalUrl("ssh://@prod:22/etc/hosts"))).rejects.toThrow(/empty username/);
});
it("matches a configured colon-suffixed alias via %3A instead of treating it as an empty port", async () => {
mockHosts([{ name: "prod:", host: "prod.internal", _source: SOURCE }]);
const spy = mockReadBytes("ok\n");
packages/coding-agent/src/internal-urls/ssh-protocol.ts
+12
View File
@@ -137,6 +137,11 @@ async function resolveTarget(url: InternalUrl, cwd?: string): Promise<SSHConnect
if (!bareHost && !rawAuthority) {
throw new Error("ssh:// requires a host: ssh://<host>/<absolute-path>");
}
if (url.password) {
throw new Error(
"ssh://: password authentication is not supported; ssh:// uses key/agent auth — drop the ':<password>' from the URL",
);
}
const isIpv6Literal = bareHost.startsWith("[") && bareHost.endsWith("]");
const sshHost = isIpv6Literal ? bareHost.slice(1, -1) : bareHost;
const username = url.username || undefined;
@@ -162,6 +167,13 @@ async function resolveTarget(url: InternalUrl, cwd?: string): Promise<SSHConnect
if (port === undefined && url.rawHost === `${username ? `${decodeOr(username)}@` : ""}${decodeOr(bareHost)}:`) {
throw new Error(`ssh://: empty port in "${url.href}"; use ssh://host:<1-65535>/<path> or drop the colon`);
}
// A literal but empty userinfo (`ssh://@host`) sets username to "" — WHATWG drops
// the `@` from hostname, but rawHost keeps the leading `@`. A percent-encoded
// alias like `%40prod` decodes to `@prod` in rawHost too, but its hostname keeps
// `%40`, so the reconstruction is `@@prod` and is left alone.
if (username === undefined && url.rawHost === `@${decodeOr(bareHost)}${port !== undefined ? `:${port}` : ""}`) {
throw new Error(`ssh://: empty username in "${url.href}"; drop the leading '@' or provide a username before it`);
}
const items = await loadConfiguredHosts(cwd);
// A literal user/port in the URL is an authority override. A configured alias