From 097e3386e08e3072cc23e73ee54f0b2a34c0d545 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Tue, 22 Sep 2026 17:27:29 +0100 Subject: [PATCH] fix(agents): keep copied Unix install commands single-line safe Use explicit shell separators instead of newline-dependent grammar for command fields. Preserve quoted values, private token files, TLS options and preflight ordering; cover paste normalization and execution failures in the owning regression suite. Refs #2123. Change-source: pulse-maintainer --- .../v6/internal/subsystems/agent-lifecycle.md | 15 ++ .../v6/internal/subsystems/api-contracts.md | 14 ++ frontend-modern/browser-verification.json | 50 ++----- .../__tests__/agentInstallCommand.test.ts | 130 +++++++++++++++++- .../src/utils/agentInstallCommand.ts | 105 +++++++------- 5 files changed, 230 insertions(+), 84 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md index 6fb3b0261e..50858dec49 100644 --- a/docs/release-control/v6/internal/subsystems/agent-lifecycle.md +++ b/docs/release-control/v6/internal/subsystems/agent-lifecycle.md @@ -8193,3 +8193,18 @@ Focused proof lives in `internal/hostagent/agent_new_test.go` `TestDefaultCollectorHostInfoKeepsErrorWhenUnrecovered` and `TestGetReliableMachineIDNormalizesWindowsBraces`); the Windows recovery file is cross-compiled with `GOOS=windows go build ./internal/hostagent/`. + +### Unix bootstrap survives single-line command fields + +The shared frontend Unix installer command uses explicit shell statement +separators rather than literal newlines or backslash-newline continuations. +This preserves its grammar when copied into a single-line command field (#2123). +The builder rejects embedded CR/LF in its normalized inputs rather than silently +altering a quoted URL, credential, CA path or option. Shell quoting is unchanged. +Download and unprivileged preflight still precede token-file creation and sudo +installation. Root and sudo paths retain private token directories, file modes, +TLS options, non-interactive operation and cleanup on success or failure. +`agentInstallCommand.test.ts` covers normalized-input syntax and +synthetic root/sudo execution under sh and Bash, including download, preflight +and install failures. A clipboard fixture proves input compatibility, not native +FreeBSD installation, offline dependency availability or service persistence. diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 327a84dd83..e3f8ae9332 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -10891,3 +10891,17 @@ setup and checks persisted non-auth preferences, rather than substituting a status-only or unrelated test for the setup contract. The registry routes only `internal/api/security_setup_fix.go` to this narrow setup proof policy; unrelated API runtime paths retain their existing verification requirements. No runtime paths or required contract updates are exempted. + +### Single-line Unix client install transport + +`frontend-modern/src/utils/agentInstallCommand.ts` emits a single-line Unix +bootstrap with explicit shell separators so text-input paste normalization +cannot join shell statements (#2123). It retains the canonical URL, quoted +credential-to-private-file transport, custom CA/insecure continuity, and +preflight-before-install ordering; no API request, response or auth scope changes. +Normalized values containing CR/LF are rejected, not rewritten. Regression +coverage in `agentInstallCommand.test.ts` checks shell syntax after +text-input normalization, literal quoted token bytes, root/sudo private-file +permissions, cleanup and early exit when download or preflight fails. This +client transport repair does not establish the reporter's native pfSense abort +cause or successful offline installation. diff --git a/frontend-modern/browser-verification.json b/frontend-modern/browser-verification.json index 08220c775c..a9ab0a57d9 100644 --- a/frontend-modern/browser-verification.json +++ b/frontend-modern/browser-verification.json @@ -1,54 +1,32 @@ { "version": 1, - "base_sha": "4b1fb511d1f1d2d363ba732914e6678f1111a9b2", - "verified_at": "2026-09-22T13:31:09.864671Z", + "base_sha": "9fbc4e7b3a8d5ddff9886852c700d43b9b6322c7", + "verified_at": "2026-09-22T16:26:06.944232Z", "result": "passed", - "changed_paths": [ - "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx", - "frontend-modern/src/components/shared/DetailSectionTable.tsx", - "frontend-modern/src/components/shared/detailSectionModel.ts" - ], + "changed_paths": ["frontend-modern/src/utils/agentInstallCommand.ts"], "content_sha256": { - "frontend-modern/src/components/Workloads/GuestDrawerOverview.tsx": "713c26cde7852d7499138684a142e623b99f591d6b1f60c5eb639efef25376ff", - "frontend-modern/src/components/shared/DetailSectionTable.tsx": "f14935b93d3062e22215f55dcf8725589265b69a60a03dae10949b374a1e2cb8", - "frontend-modern/src/components/shared/detailSectionModel.ts": "0669d3afc5bc3d0809c3c2ee4204913c28aa87703c218ee3110b626e5792d086" + "frontend-modern/src/utils/agentInstallCommand.ts": "5c456b9b8cf2d114d5be62bb46c27b265d30fc49c3df88d2ec42631414f65c64" }, - "routes": [ - "GuestDrawerOverview production-component fixture /proof.html" - ], + "routes": ["Production buildUnixAgentInstallCommand in synthetic clipboard/text-input fixture"], "viewports": [ - { - "width": 1600, - "height": 1100 - }, { "width": 1280, - "height": 1100 - }, - { - "width": 768, - "height": 1100 + "height": 850 }, { "width": 390, - "height": 1100 - }, - { - "width": 320, - "height": 1100 + "height": 850 } ], "states": [ - "Guest with System, Memory, Filesystems and Tags sections", - "Shared-prefix mount paths ending in Films and Series", - "Long path with spaces and 140-character unbroken path segment", - "1280px desktop with a constrained 650px pane; tablet/phone touch emulation" + "Tokenless HTTPS, token HTTP, quoted URL/token, insecure HTTPS, custom CA with extra flags" ], "interactions": [ - "Read all paths without hovering or scrolling horizontally", - "Assert text ranges fit all five label boxes at all five layouts, no page overflow or uncaught errors, and five progress bars", - "Inspect before/after desktop, constrained desktop and smallest-phone screenshots" + "Copy generated command with browser clipboard API and paste with Control+V into input type=text", + "Assert five variants preserve exact bytes at both viewports; old source changes all five", + "Check pasted output syntax with sh and bash: old five fail, repaired five pass", + "Inspect desktop-before and narrow-after screenshots" ], - "command": "pulse-worker-browser .proof-filesystems/after.cjs", - "notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. Synthetic guest, real GuestDrawerOverview, shared renderer and application CSS. Before repair, desktop labels are 112px with ellipsis; after repair complete paths wrap above usage. No live backend, published-build qualification or reporter confirmation. Other detail rows retain their compact layout." + "command": "pulse-worker-browser .proof-install/browser.cjs", + "notes": "Chromium 141.0.7390.37 / Playwright 1.56.1. TypeScript-transpiled production utility, synthetic pulse.invalid and invented tokens. Generic text field, not pfSense DOM or installation. No backend/network/service proof. Full fixture and results retained with lane outcome." } diff --git a/frontend-modern/src/utils/__tests__/agentInstallCommand.test.ts b/frontend-modern/src/utils/__tests__/agentInstallCommand.test.ts index a71a997823..15ba45ea15 100644 --- a/frontend-modern/src/utils/__tests__/agentInstallCommand.test.ts +++ b/frontend-modern/src/utils/__tests__/agentInstallCommand.test.ts @@ -1,7 +1,7 @@ import { execFileSync } from 'node:child_process'; -import { chmodSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { chmodSync, existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; -import { join } from 'node:path'; +import { dirname, join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { buildUnixAgentInstallCommand, @@ -394,3 +394,129 @@ describe('resolveAgentCommandPlatform', () => { expect(resolveAgentCommandPlatform(null)).toBe('linux'); }); }); + +const variants = [ + { baseUrl: 'https://pulse.invalid' }, + { baseUrl: 'http://pulse.invalid/base/', token: 'invented-token' }, + { baseUrl: "https://pulse.invalid/agent's path", token: "tok'en;$(false)`false`\\value" }, + { baseUrl: 'https://pulse.invalid', insecure: true, token: 'invented-token' }, + { + baseUrl: 'https://pulse.invalid', + caCertPath: "/tmp/agent's ca.pem", + extraArgs: ['--enable-docker', '--disable-host'], + }, +]; + +describe('single-line Unix install commands', () => { + it.each(variants)('survives text-input normalization: %j', (options) => { + const command = buildUnixAgentInstallCommand(options); + const input = document.createElement('input'); + input.type = 'text'; + input.value = command; + expect(command).not.toMatch(/[\r\n]/); + expect(input.value).toBe(command); + for (const shell of ['sh', 'bash']) { + expect(() => execFileSync(shell, ['-n', '-c', input.value])).not.toThrow(); + } + }); + + it.each([ + { baseUrl: 'https://pulse.invalid/a\nb' }, + { baseUrl: 'https://pulse.invalid', token: 'a\rb' }, + { baseUrl: 'https://pulse.invalid', caCertPath: '/tmp/a\nb' }, + { baseUrl: 'https://pulse.invalid', extraArgs: ['--a\n--b'] }, + ])('rejects embedded line breaks without silently changing values: %j', (options) => { + expect(() => buildUnixAgentInstallCommand(options)).toThrow('must not contain line breaks'); + }); + + it.each(['sh', 'bash'])( + 'preserves execution and failure ordering under %s after paste', + (shell) => { + const dir = mkdtempSync(join(tmpdir(), 'pulse-single-line-')); + const trace = join(dir, 'trace'); + const tokenPath = join(dir, 'token-path'); + const scriptPath = join(dir, 'script-path'); + const token = "tok'en;$(false)`false`\\value"; + const installer = join(dir, 'fixture-installer'); + const fake = (name: string, body: string) => { + writeFileSync(join(dir, name), `#!/bin/sh\n${body}\n`); + chmodSync(join(dir, name), 0o700); + }; + try { + fake('id', 'echo "$FAKE_UID"'); + fake('sudo', 'echo sudo >> "$TRACE"; exec "$@"'); + fake( + 'curl', + `echo fetch >> "$TRACE" +while [ "$#" -gt 0 ]; do + if [ "$1" = -o ]; then output="$2"; shift; fi + shift +done +printf %s "$output" > "$SCRIPT_PATH" +[ "$FAIL_AT" != fetch ] || exit 22 +cp "$INSTALLER" "$output"`, + ); + writeFileSync( + installer, + `#!/bin/sh +set -e +case " $* " in + *' --preflight-only '*) echo preflight >> "$TRACE"; [ "$FAIL_AT" != preflight ]; exit $? ;; +esac +echo install >> "$TRACE" +while [ "$#" -gt 0 ]; do + if [ "$1" = --token-file ]; then token_file="$2"; shift; fi + shift +done +printf %s "$token_file" > "$TOKEN_PATH" +[ "$(stat -c %a "$token_file")" = 600 ] +[ "$(stat -c %a "$(dirname "$token_file")")" = 700 ] +[ "$(cat "$token_file")" = "$EXPECTED_TOKEN" ] +[ "$FAIL_AT" != install ] +`, + ); + for (const uid of ['0', '1000']) { + for (const failAt of ['', 'fetch', 'preflight', 'install']) { + writeFileSync(trace, ''); + rmSync(tokenPath, { force: true }); + const command = buildUnixAgentInstallCommand({ + baseUrl: 'https://pulse.invalid', + token, + }); + const input = document.createElement('input'); + input.value = command; + const run = () => + execFileSync(shell, ['-c', input.value], { + env: { + ...process.env, + PATH: `${dir}:${process.env.PATH}`, + FAKE_UID: uid, + FAIL_AT: failAt, + TRACE: trace, + TOKEN_PATH: tokenPath, + SCRIPT_PATH: scriptPath, + INSTALLER: installer, + EXPECTED_TOKEN: token, + }, + }); + if (failAt) expect(run).toThrow(); + else expect(run).not.toThrow(); + const events = readFileSync(trace, 'utf8').trim().split('\n'); + expect(events[0]).toBe('fetch'); + expect(existsSync(dirname(readFileSync(scriptPath, 'utf8')))).toBe(false); + if (failAt === 'fetch' || failAt === 'preflight') { + expect(events).toEqual(failAt === 'fetch' ? ['fetch'] : ['fetch', 'preflight']); + expect(existsSync(tokenPath)).toBe(false); + } else { + expect(events[1]).toBe('preflight'); + expect(events).toContain('install'); + expect(existsSync(dirname(readFileSync(tokenPath, 'utf8')))).toBe(false); + } + } + } + } finally { + rmSync(dir, { recursive: true, force: true }); + } + }, + ); +}); diff --git a/frontend-modern/src/utils/agentInstallCommand.ts b/frontend-modern/src/utils/agentInstallCommand.ts index b63e911078..1a38ceb454 100644 --- a/frontend-modern/src/utils/agentInstallCommand.ts +++ b/frontend-modern/src/utils/agentInstallCommand.ts @@ -148,65 +148,78 @@ export const buildUnixAgentInstallCommand = ({ const normalizedExtraArgs = extraArgs.map((arg) => arg.trim()).filter((arg) => arg.length > 0); const installRequiresInsecure = insecure || normalizedBaseUrl.startsWith('http://'); const curlFlags = insecure ? '-kfsSL' : '-fsSL'; - const caCertArg = normalizedCaCertPath - ? ` \\\n --cacert ${shellQuoteArg(normalizedCaCertPath)}` - : ''; - const insecureArg = installRequiresInsecure ? ` \\\n --insecure` : ''; + // Build shell grammar explicitly: command fields such as pfSense's remove + // pasted newlines. Never flatten a multiline script (or quoted user data). + if ( + [normalizedBaseUrl, normalizedToken, normalizedCaCertPath, ...normalizedExtraArgs].some( + (value) => /[\r\n]/.test(value), + ) + ) { + throw new Error('Pulse install command values must not contain line breaks.'); + } + const caCertArg = normalizedCaCertPath ? ` --cacert ${shellQuoteArg(normalizedCaCertPath)}` : ''; + const insecureArg = installRequiresInsecure ? ' --insecure' : ''; const preflightArgs = [ `--url ${shellQuoteArg(normalizedBaseUrl)}`, '--preflight-only', '--output json', '--non-interactive', - ].join(' \\\n '); + ].join(' '); const installArgs = [ `--url ${shellQuoteArg(normalizedBaseUrl)}`, ...(normalizedToken ? ['--token-file "$token_file"'] : []), ...normalizedExtraArgs, '--non-interactive', - ].join(' \\\n '); + ].join(' '); const rootTokenSetup = normalizedToken - ? ` token_dir=$(mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX) - token_file="$token_dir/token" - umask 077 - printf %s ${shellQuoteArg(normalizedToken)} > "$token_file" -` - : ''; + ? [ + 'token_dir=$(mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX);', + 'token_file="$token_dir/token";', + 'umask 077;', + `printf %s ${shellQuoteArg(normalizedToken)} > "$token_file";`, + ] + : []; const sudoTokenSetup = normalizedToken - ? ` token_dir=$(sudo mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX) - token_file="$token_dir/token" - printf %s ${shellQuoteArg(normalizedToken)} | sudo tee "$token_file" >/dev/null - sudo chmod 0600 "$token_file" -` - : ''; + ? [ + 'token_dir=$(sudo mktemp -d /tmp/pulse-agent-bootstrap.XXXXXX);', + 'token_file="$token_dir/token";', + `printf %s ${shellQuoteArg(normalizedToken)} | sudo tee "$token_file" >/dev/null;`, + 'sudo chmod 0600 "$token_file";', + ] + : []; - return `( - set -e - tmp_dir=$(mktemp -d) - token_dir="" - install_script="$tmp_dir/install.sh" - cleanup() { - rm -rf -- "$tmp_dir" - if [ -n "${'${token_dir:-}'}" ]; then - if [ "$(id -u)" -eq 0 ]; then - rm -rf -- "$token_dir" - elif command -v sudo >/dev/null 2>&1; then - sudo rm -rf -- "$token_dir" >/dev/null 2>&1 || true - fi - fi - } - trap cleanup EXIT HUP INT TERM - curl ${curlFlags}${normalizedCaCertPath ? ` --cacert ${shellQuoteArg(normalizedCaCertPath)}` : ''} ${shellQuoteArg(`${normalizedBaseUrl}/install.sh`)} -o "$install_script" - chmod +x "$install_script" - bash "$install_script" ${preflightArgs}${caCertArg}${insecureArg} - if [ "$(id -u)" -eq 0 ]; then -${rootTokenSetup} bash "$install_script" ${installArgs}${caCertArg}${insecureArg} - elif command -v sudo >/dev/null 2>&1; then -${sudoTokenSetup} sudo bash "$install_script" ${installArgs}${caCertArg}${insecureArg} - else - echo "Root privileges required. Run as root (su -) and retry." >&2 - exit 1 - fi -)`; + return [ + '(', + 'set -e;', + 'tmp_dir=$(mktemp -d);', + 'token_dir="";', + 'install_script="$tmp_dir/install.sh";', + 'cleanup() {', + 'rm -rf -- "$tmp_dir";', + 'if [ -n "${token_dir:-}" ]; then', + 'if [ "$(id -u)" -eq 0 ]; then', + 'rm -rf -- "$token_dir";', + 'elif command -v sudo >/dev/null 2>&1; then', + 'sudo rm -rf -- "$token_dir" >/dev/null 2>&1 || true;', + 'fi;', + 'fi;', + '};', + 'trap cleanup EXIT HUP INT TERM;', + `curl ${curlFlags}${caCertArg} ${shellQuoteArg(`${normalizedBaseUrl}/install.sh`)} -o "$install_script";`, + 'chmod +x "$install_script";', + `bash "$install_script" ${preflightArgs}${caCertArg}${insecureArg};`, + 'if [ "$(id -u)" -eq 0 ]; then', + ...rootTokenSetup, + `bash "$install_script" ${installArgs}${caCertArg}${insecureArg};`, + 'elif command -v sudo >/dev/null 2>&1; then', + ...sudoTokenSetup, + `sudo bash "$install_script" ${installArgs}${caCertArg}${insecureArg};`, + 'else', + 'echo "Root privileges required. Run as root (su -) and retry." >&2;', + 'exit 1;', + 'fi;', + ')', + ].join(' '); }; export const buildPowerShellInstallScriptBootstrap = (baseUrl: string) => {