mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 23:19:43 +02:00
feat(remote): add the password field to the remote-host form
The backend could already authenticate with a password, but only the API could supply the field — the UI had no way in. It goes in "Advanced SSH" next to Identity File: the two are answers to the same question, and seeing them together is what makes the choice obvious. The hint says outright to prefer a key when one exists. Three details, each of which breaks something if skipped: - the field is type="password" and is never populated from the server. GET is redacted, so any code writing a value into this box can only be writing a placeholder — and the next save would store that placeholder as the real password. - the value is deliberately not trimmed: leading or trailing spaces may be part of the password. - it is added to the remoteFields clearing list. Without that, a typed password persists across forms and the next new host silently inherits it — credentials from two different machines bleeding together. Both submit paths are wired: the standalone "add remote host", and the one that creates a host as part of the remote-case flow. Wiring only one leaves the other silently key-only. Guard tests pin each of the above (including "must be both paths" and "must never repopulate").
This commit is contained in:
@@ -2818,6 +2818,11 @@
|
||||
<input type="text" id="remoteHostIdentityFile" placeholder="~/.ssh/remote_ed25519" autocomplete="off" autocapitalize="off" autocorrect="off" spellcheck="false">
|
||||
<span class="form-hint">Optional. Path to a private key on this machine (passed to ssh -i). Never the key contents.</span>
|
||||
</div>
|
||||
<div class="form-row">
|
||||
<label>Password</label>
|
||||
<input type="password" id="remoteHostPassword" placeholder="leave blank to use a key" autocomplete="new-password" autocapitalize="off" autocorrect="off" spellcheck="false">
|
||||
<span class="form-hint">Optional, for hosts that accept no key. Stored on this machine only (<code>remote-hosts.json</code>, mode 0600), never sent back to the browser, and handed to ssh through <code>sshpass</code>'s SSHPASS variable rather than the command line. Needs <code>sshpass</code> installed here. Prefer a key when the host allows one.</span>
|
||||
</div>
|
||||
<div class="form-row">
|
||||
<label>SOCKS Proxy</label>
|
||||
<input type="text" id="remoteHostSocksProxy" placeholder="127.0.0.1:1080" autocomplete="off" autocapitalize="off" spellcheck="false">
|
||||
|
||||
@@ -2466,6 +2466,9 @@ Object.assign(CodemanApp.prototype, {
|
||||
'remoteHostPort',
|
||||
'remoteHostCodexCommand',
|
||||
'remoteHostIdentityFile',
|
||||
// ⚠️ Must be cleared with the rest: a password left in the field would be
|
||||
// silently inherited by the NEXT host created from this form.
|
||||
'remoteHostPassword',
|
||||
'remoteHostSocksProxy',
|
||||
'remoteHostJumpHost',
|
||||
'remoteHostExtraSshOptions',
|
||||
@@ -3049,6 +3052,8 @@ Object.assign(CodemanApp.prototype, {
|
||||
// COD-107 — port + advanced SSH connection options.
|
||||
const portRaw = document.getElementById('remoteHostPort').value.trim();
|
||||
const identityFile = document.getElementById('remoteHostIdentityFile').value.trim();
|
||||
// Deliberately NOT trimmed: leading/trailing spaces can be part of a password.
|
||||
const password = document.getElementById('remoteHostPassword').value;
|
||||
const socksProxy = document.getElementById('remoteHostSocksProxy').value.trim();
|
||||
const jumpHost = document.getElementById('remoteHostJumpHost').value.trim();
|
||||
const extraSshOptions = document.getElementById('remoteHostExtraSshOptions').value
|
||||
@@ -3085,6 +3090,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
username,
|
||||
...(port ? { port } : {}),
|
||||
...(identityFile ? { identityFile } : {}),
|
||||
...(password ? { password } : {}),
|
||||
...(socksProxy ? { socksProxy } : {}),
|
||||
...(jumpHost ? { jumpHost } : {}),
|
||||
...(extraSshOptions.length ? { extraSshOptions } : {}),
|
||||
@@ -3417,6 +3423,8 @@ Object.assign(CodemanApp.prototype, {
|
||||
const username = document.getElementById('remoteHostUsername').value.trim();
|
||||
const portRaw = document.getElementById('remoteHostPort').value.trim();
|
||||
const identityFile = document.getElementById('remoteHostIdentityFile').value.trim();
|
||||
// Deliberately NOT trimmed: leading/trailing spaces can be part of a password.
|
||||
const password = document.getElementById('remoteHostPassword').value;
|
||||
const socksProxy = document.getElementById('remoteHostSocksProxy').value.trim();
|
||||
const jumpHost = document.getElementById('remoteHostJumpHost').value.trim();
|
||||
const codexCommand = document.getElementById('remoteHostCodexCommand').value.trim();
|
||||
@@ -3436,6 +3444,7 @@ Object.assign(CodemanApp.prototype, {
|
||||
username,
|
||||
...(port ? { port } : {}),
|
||||
...(identityFile ? { identityFile } : {}),
|
||||
...(password ? { password } : {}),
|
||||
...(socksProxy ? { socksProxy } : {}),
|
||||
...(jumpHost ? { jumpHost } : {}),
|
||||
...(extraSshOptions.length ? { extraSshOptions } : {}),
|
||||
|
||||
@@ -5,10 +5,10 @@
|
||||
* environment variable, never argv), and the variable itself is injected into
|
||||
* the pane with socket-scoped `tmux setenv` like every other secret here.
|
||||
*/
|
||||
import { promises as fs } from 'node:fs';
|
||||
import { promises as fs, readFileSync } from 'node:fs';
|
||||
import { mkdtempSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { join } from 'node:path';
|
||||
import { join, resolve } from 'node:path';
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import {
|
||||
buildSshConnectionArgs,
|
||||
@@ -108,3 +108,42 @@ describe('remote-hosts.json holds a secret, so it is 0600', () => {
|
||||
expect((await fs.stat(path)).mode & 0o777).toBe(0o600);
|
||||
});
|
||||
});
|
||||
|
||||
describe('the remote-host form wires the password without leaking it', () => {
|
||||
const html = readFileSync(resolve(import.meta.dirname, '../src/web/public/index.html'), 'utf8');
|
||||
const ui = readFileSync(resolve(import.meta.dirname, '../src/web/public/session-ui.js'), 'utf8');
|
||||
|
||||
it('renders a masked field, never a plain text input', () => {
|
||||
const field = /<input[^>]*id="remoteHostPassword"[^>]*>/.exec(html)?.[0] ?? '';
|
||||
expect(field, 'remoteHostPassword must exist').not.toBe('');
|
||||
expect(field).toContain('type="password"');
|
||||
});
|
||||
|
||||
it('sends the password on BOTH submit paths', () => {
|
||||
// The form is reachable from "add a remote host" AND from the remote-case
|
||||
// create flow, which builds its own host payload; wiring only one leaves the
|
||||
// other silently key-only.
|
||||
expect(ui.match(/\.\.\.\(password \? \{ password \} : \{\}\),/g) ?? []).toHaveLength(2);
|
||||
expect(ui.match(/getElementById\('remoteHostPassword'\)/g) ?? []).toHaveLength(2);
|
||||
});
|
||||
|
||||
it('does not trim the value, since spaces can be part of a password', () => {
|
||||
for (const m of ui.matchAll(/const password = document\.getElementById\('remoteHostPassword'\)\.value([^;]*);/g)) {
|
||||
expect(m[1]).toBe('');
|
||||
}
|
||||
});
|
||||
|
||||
it('clears the field with the rest of the form', () => {
|
||||
// Left behind, a typed password would be inherited by the NEXT host created
|
||||
// from this form — a real leak between two different machines' credentials.
|
||||
const list = /const remoteFields = \[([\s\S]*?)\];/.exec(ui)?.[1] ?? '';
|
||||
expect(list, 'remoteFields list not found').not.toBe('');
|
||||
expect(list).toContain("'remoteHostPassword'");
|
||||
});
|
||||
|
||||
it('never populates the field from a server response', () => {
|
||||
// GET redacts the password, so anything assigning to this field would be
|
||||
// writing a placeholder that a later save would persist as the real value.
|
||||
expect(ui).not.toMatch(/getElementById\('remoteHostPassword'\)\.value\s*=/);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user