From e0f9434eafca4bba8cc520223ea13da06da6904d Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Mon, 4 May 2026 19:58:49 -0700 Subject: [PATCH] fix(files-eic): silence ssh known-hosts warning that 500'd Hermes config load MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GET /workspaces/:id/files/config.yaml on hongming.moleculesai.app's Hermes workspace returned 500 with body: ssh cat: exit status 1 (Warning: Permanently added '[127.0.0.1]:37951' (ED25519) to the list of known hosts.) Root cause: ssh emits the "Permanently added" notice on every fresh tunnel connection, even with UserKnownHostsFile=/dev/null (that prevents persistence, not the warning). It lands on stderr, fooling readFileViaEIC's classifier: if len(out) == 0 && stderr.Len() == 0 { return nil, os.ErrNotExist } return nil, fmt.Errorf("ssh cat: %w (%s)", runErr, ...) stderr was non-empty (the warning), so we returned the wrapped error → 500 from the HTTP layer instead of 404. Fix: add `-o LogLevel=ERROR` to BOTH writeFileViaEIC and readFileViaEIC ssh invocations. Silences info+warning while keeping real auth/tunnel errors visible (those emit at ERROR level). Test: TestSSHArgs_LogLevelErrorBothSites pins the flag in both blocks. Mutation-tested: stripping the flag from one site fails the gate. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../internal/handlers/template_files_eic.go | 20 ++++++++++++ .../handlers/template_files_eic_test.go | 32 +++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/workspace-server/internal/handlers/template_files_eic.go b/workspace-server/internal/handlers/template_files_eic.go index da8a7d14..edce34fc 100644 --- a/workspace-server/internal/handlers/template_files_eic.go +++ b/workspace-server/internal/handlers/template_files_eic.go @@ -178,6 +178,16 @@ func writeFileViaEIC(ctx context.Context, instanceID, runtime, relPath string, c "-i", keyPath, "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + // LogLevel=ERROR silences the benign "Warning: Permanently + // added '[127.0.0.1]:NNNNN' to known hosts" notice that ssh + // emits on every fresh tunnel connection. Without this, the + // notice lands on stderr and fools readFileViaEIC's "empty + // stdout + empty stderr → file not found" classifier into + // thinking the warning is a real ssh-layer error → 500 + // instead of 404 (Hermes config.yaml load, hongming tenant, + // 2026-05-05 02:38). Real auth/tunnel errors stay visible + // because they're emitted at ERROR level. + "-o", "LogLevel=ERROR", "-o", "ServerAliveInterval=15", "-p", fmt.Sprintf("%d", localPort), fmt.Sprintf("%s@127.0.0.1", osUser), @@ -292,6 +302,16 @@ func readFileViaEIC(ctx context.Context, instanceID, runtime, relPath string) ([ "-i", keyPath, "-o", "StrictHostKeyChecking=no", "-o", "UserKnownHostsFile=/dev/null", + // LogLevel=ERROR silences the benign "Warning: Permanently + // added '[127.0.0.1]:NNNNN' to known hosts" notice that ssh + // emits on every fresh tunnel connection. Without this, the + // notice lands on stderr and fools readFileViaEIC's "empty + // stdout + empty stderr → file not found" classifier into + // thinking the warning is a real ssh-layer error → 500 + // instead of 404 (Hermes config.yaml load, hongming tenant, + // 2026-05-05 02:38). Real auth/tunnel errors stay visible + // because they're emitted at ERROR level. + "-o", "LogLevel=ERROR", "-o", "ServerAliveInterval=15", "-p", fmt.Sprintf("%d", localPort), fmt.Sprintf("%s@127.0.0.1", osUser), diff --git a/workspace-server/internal/handlers/template_files_eic_test.go b/workspace-server/internal/handlers/template_files_eic_test.go index 30bd9988..e5bc8a48 100644 --- a/workspace-server/internal/handlers/template_files_eic_test.go +++ b/workspace-server/internal/handlers/template_files_eic_test.go @@ -1,6 +1,8 @@ package handlers import ( + "os" + "regexp" "strings" "testing" ) @@ -66,6 +68,36 @@ func TestResolveWorkspaceFilePath_RejectsTraversal(t *testing.T) { } } +// TestSSHArgs_LogLevelErrorBothSites pins that BOTH ssh invocations +// (writeFileViaEIC + readFileViaEIC) include `-o LogLevel=ERROR`. +// +// Without that flag, ssh emits a "Warning: Permanently added +// '[127.0.0.1]:NNNNN' (ED25519) to the list of known hosts." line on +// every fresh tunnel connection (even with UserKnownHostsFile=/dev/null +// — that prevents persistence, not the warning). The warning lands on +// stderr, which fools readFileViaEIC's "empty stdout + empty stderr → +// file not found" classifier into thinking the warning is a real +// ssh-layer error and returning 500 instead of 404. +// +// Caught 2026-05-05 02:38 on hongming.moleculesai.app: opening Hermes +// workspace's Config tab returned 500 with body +// `ssh cat: exit status 1 (Warning: Permanently added '[127.0.0.1]:37951'…)`. +// +// LogLevel=ERROR silences info+warning while keeping real auth/tunnel +// errors visible. This test reads the source and asserts the flag +// appears at least twice (one per ssh block) — fires if a future edit +// removes it from either site. +func TestSSHArgs_LogLevelErrorBothSites(t *testing.T) { + src, err := os.ReadFile("template_files_eic.go") + if err != nil { + t.Fatalf("read source: %v", err) + } + matches := regexp.MustCompile(`"-o", "LogLevel=ERROR"`).FindAllIndex(src, -1) + if len(matches) < 2 { + t.Errorf("expected LogLevel=ERROR in BOTH ssh blocks (write + read); found %d occurrences", len(matches)) + } +} + // TestShellQuote — the sole piece of variable data in the remote ssh // command is the absolute path. It's already built from a map + Clean() // so traversal is impossible, but we still single-quote as defence-in-