-
Notifications
You must be signed in to change notification settings - Fork 0
Fix fresh Linux connector and Windows reboot setup #50
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import assert from "node:assert/strict"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { chmod, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promises"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { readFileSync } from "node:fs"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { tmpdir } from "node:os"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import { join } from "node:path"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -17,6 +17,33 @@ test("Linux release packaging ships pinned connectors for every supported host a | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.match(packageLinux, /cloudflared-linux-\$\{connector\.arch\}/, "the verified binaries enter the Linux release archive"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.match(packageLinux, /chmodSync\(destination, 0o755\)/, "packaged Linux connectors retain executable mode"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.match(resolver, /cloudflared-linux-\$\{process\.arch\}/, "Linux resolves only the binary matching the running host architecture"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.match(resolver, /process\.env\.HELM_APP_ROOT[\s\S]*process\.cwd\(\)/, "an installed Linux service can resolve its bundled connector from its release working directory"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("Linux service working directory resolves the bundled connector without a legacy app-root environment", async (t) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const root = await mkdtemp(join(tmpdir(), "1helm-linux-connector-root-")); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const resources = join(root, "resources"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const binary = join(resources, `cloudflared-linux-${process.arch}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const originalCwd = process.cwd(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const originalAppRoot = process.env.HELM_APP_ROOT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const originalResources = process.env.HELM_RESOURCES_PATH; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const originalBinary = process.env.CLOUDFLARED_BIN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await mkdir(resources); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await writeFile(binary, "#!/bin/sh\nexit 0\n"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await chmod(binary, 0o755); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.chdir(root); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| delete process.env.HELM_APP_ROOT; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| delete process.env.HELM_RESOURCES_PATH; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| delete process.env.CLOUDFLARED_BIN; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| t.after(async () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| process.chdir(originalCwd); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (originalAppRoot === undefined) delete process.env.HELM_APP_ROOT; else process.env.HELM_APP_ROOT = originalAppRoot; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (originalResources === undefined) delete process.env.HELM_RESOURCES_PATH; else process.env.HELM_RESOURCES_PATH = originalResources; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (originalBinary === undefined) delete process.env.CLOUDFLARED_BIN; else process.env.CLOUDFLARED_BIN = originalBinary; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await rm(root, { recursive: true, force: true }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const connectors = await import(`../src/server/connectors.ts?linux-root-test=${Date.now()}`); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.equal(connectors.connectorAvailable(), true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(connectors\.mjs|connectors\.ts|package\.json|vitest|jest|node:test|test)' | head -200
printf '%s\n' '--- test file outline ---'
ast-grep outline test/connectors.mjs --lang javascript 2>/dev/null || true
printf '%s\n' '--- relevant test section ---'
cat -n test/connectors.mjs | sed -n '1,90p'
printf '%s\n' '--- connector resolver symbols ---'
rg -n -C 8 'connectorBinary|linuxConnector|connectorAvailable|HELM_APP_ROOT|HELM_RESOURCES_PATH|CLOUDFLARED_BIN|process\.cwd|PATH' src test package.jsonRepository: gitcommit90/1Helm Length of output: 50373 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- connector resolver ---'
cat -n src/server/connectors.ts | sed -n '1,75p'
printf '%s\n' '--- package test commands ---'
cat -n package.json | sed -n '1,140p'
printf '%s\n' '--- connector test references and platform guards ---'
rg -n -C 3 'test\(|process\.platform|t\.skip|PATH|HELM_RESOURCES_PATH|CLOUDFLARED_BIN' test/connectors.mjsRepository: gitcommit90/1Helm Length of output: 13666 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- test suite runner ---'
cat -n scripts/run-test-suite.mjs | sed -n '1,180p'
printf '%s\n' '--- resolver behavior probe ---'
python3 - <<'PY'
from pathlib import Path
text = Path("src/server/connectors.ts").read_text()
start = text.index("function connectorBinary()")
end = text.index("\n\nconst safeId", start)
print(text[start:end])
PY
node - <<'JS'
const path = require('node:path');
function candidates({platform, arch, env, cwd}) {
const join = path.posix.join;
const resources = env.HELM_RESOURCES_PATH || "";
const appRoots = [...new Set([env.HELM_APP_ROOT || "", cwd].filter(Boolean))];
const pathSep = platform === "win32" ? ";" : ":";
const pathNames = platform === "win32" ? ["cloudflared.exe", "cloudflared"] : ["cloudflared"];
const linuxConnector = platform === "linux" && (arch === "x64" || arch === "arm64")
? `cloudflared-linux-${arch}` : "";
const pathCandidates = String(env.PATH || "").split(pathSep).filter(Boolean)
.flatMap(directory => pathNames.map(name => join(directory, name)));
return [
env.CLOUDFLARED_BIN || "",
resources ? join(resources, "cloudflared.exe") : "",
resources ? join(resources, "cloudflared") : "",
...appRoots.flatMap(appRoot => [
join(appRoot, "cloudflared.exe"),
join(appRoot, "cloudflared"),
linuxConnector ? join(appRoot, "resources", linuxConnector) : "",
]),
"/opt/homebrew/bin/cloudflared", "/usr/local/bin/cloudflared",
"/usr/bin/cloudflared", ...pathCandidates,
].filter(Boolean);
}
for (const [platform, arch] of [["linux","x64"],["linux","arm64"],["linux","ia32"],["darwin","arm64"],["win32","x64"]]) {
const list = candidates({platform, arch, env:{}, cwd:"/tmp/root"});
console.log(platform, arch, list.includes(`/tmp/root/resources/cloudflared-linux-${arch}`), list);
}
JSRepository: gitcommit90/1Helm Length of output: 6660 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- workflow platforms and connector test invocation ---'
rg -n -C 3 'npm test|test:|runs-on:|node --test|connectors\.mjs' .github site scripts 2>/dev/null | head -240 || true
printf '%s\n' '--- node test skip API availability ---'
node - <<'JS'
import test from "node:test";
test("skip-api-probe", (t) => {
console.log({hasSkip: typeof t.skip === "function"});
t.skip("probe");
});
JSRepository: gitcommit90/1Helm Length of output: 2766 Skip this Linux-only fixture on unsupported runners.
Proposed fix test("Linux service working directory resolves the bundled connector without a legacy app-root environment", async (t) => {
+ if (process.platform !== "linux" || (process.arch !== "x64" && process.arch !== "arm64")) {
+ t.skip("Linux connector fixture requires a supported Linux architecture");
+ return;
+ }
const root = await mkdtemp(join(tmpdir(), "1helm-linux-connector-root-"));📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| test("stopping a connector cancels automatic relaunch while preserving its credentials", async (t) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
|
|
@@ -194,6 +194,10 @@ test("desktop entrypoint keeps the renderer sandboxed and data on the Mac", asyn | |||||||
| assert.match(windowsRuntime, /if \(\$HostSetup\)[\s\S]*try \{[\s\S]*catch \{[\s\S]*Fail-Setup \$message/, "unexpected elevated host-setup errors are written to shared status instead of collapsing to an unexplained exit code"); | ||||||||
| assert.match(windowsRuntime, /StatusPath/, "elevated HostSetup receives the shared status path so real errors reach the app"); | ||||||||
| assert.match(windowsRuntime, /1603[\s\S]*Test-PinnedWslRuntime|Test-PinnedWslRuntime[\s\S]*1603/, "MSI 1603 falls back to re-verifying an already-present pinned WSL runtime"); | ||||||||
| assert.match(windowsRuntime, /\$enabledWslFeatureNow[\s\S]*\$enabledVmFeatureNow[\s\S]*\$restartRequired/, "features enabled in the current pass force a reboot before WSL import regardless of DISM enum formatting"); | ||||||||
| assert.match(windowsRuntime, /Get-Service -Name vmcompute[\s\S]*\$restartRequired = \$true/, "an enabled-but-not-registered WSL VM compute service stops setup at the reboot boundary"); | ||||||||
| assert.match(windowsRuntime, /HCS_E_SERVICE_NOT_AVAILABLE[\s\S]*Require-WindowsRestart|Test-WslRestartFailure[\s\S]*Require-WindowsRestart/, "an unavailable VM compute service is reported as restart-required instead of a broken runtime"); | ||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Target the restart-routing call sites. The expression can match Proposed test update- assert.match(windowsRuntime, /HCS_E_SERVICE_NOT_AVAILABLE[\s\S]*Require-WindowsRestart|Test-WslRestartFailure[\s\S]*Require-WindowsRestart/, "an unavailable VM compute service is reported as restart-required instead of a broken runtime");
+ assert.match(windowsRuntime, /if \(Test-WslRestartFailure \$defaultVersion\.Text\) \{ Require-WindowsRestart \}/, "a default-version HCS failure requires a Windows restart");
+ assert.match(windowsRuntime, /if \(Test-WslRestartFailure \$imported\.Text\) \{ Require-WindowsRestart \}/, "an import HCS failure requires a Windows restart");📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||
| assert.match(windowsRuntime, /\.1helm-partial-import[\s\S]*\$ownedPartial[\s\S]*Remove-Item -LiteralPath \$installDirectory/, "an app-owned partial WSL import can recover safely after reboot"); | ||||||||
| assert.match(windowsRuntime, /\[automount\][\s\S]*enabled=false[\s\S]*\[interop\][\s\S]*enabled=false/, "the shared runtime exposes neither Windows drives nor process interop"); | ||||||||
| assert.doesNotMatch(windowsRuntime, /--update/); | ||||||||
| assert.match(channelComputers, /HELM_WSL_SETUP_STATUS/, "Windows runtime install is tracked through a status file instead of fire-and-forget Start-Process"); | ||||||||
|
|
||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: gitcommit90/1Helm
Length of output: 17107
🏁 Script executed:
Repository: gitcommit90/1Helm
Length of output: 9771
Guard
process.cwd()before building candidates.If the current directory was removed,
process.cwd()throwsENOENT. This preventsconnectorAvailable()and connector launches from checking configured or system paths. Read it in atry/catchand omit it when unavailable.🧰 Tools
🪛 ast-grep (0.45.0)
[warning] Importing child_process exposes a command-execution surface; ensure any command/argument built from input is validated, and prefer execFile/spawn with an argument array over exec.
Context: import { spawn, type ChildProcess } from "node:child_process";
Note: [CWE-78] Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection').
(detect-child-process-typescript)
🤖 Prompt for AI Agents