Skip to content

Commit d0e7f64

Browse files
Copilotlpcox
andauthored
Address review: make GITHUB_ENV writes best-effort and rename test case
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/8c6049c3-8a7e-4abf-93b8-615d1b4fb45a Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent f274154 commit d0e7f64

2 files changed

Lines changed: 12 additions & 3 deletions

File tree

internal/cmd/proxy.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ func configureTLSTrustEnvironment(caCertPath string) error {
335335
}
336336

337337
// Best-effort append: the proxy should still start even if GITHUB_ENV cannot be opened.
338-
f, err := os.OpenFile(githubEnvPath, os.O_APPEND|os.O_WRONLY, 0o644)
338+
f, err := os.OpenFile(githubEnvPath, os.O_APPEND|os.O_WRONLY, 0)
339339
if err != nil {
340340
logger.LogWarn("startup", "Skipping GITHUB_ENV TLS trust export: open failed for %s: %v", githubEnvPath, err)
341341
return nil
@@ -344,7 +344,8 @@ func configureTLSTrustEnvironment(caCertPath string) error {
344344

345345
for _, key := range tlsTrustEnvKeys {
346346
if _, err := io.WriteString(f, key+"="+caCertPath+"\n"); err != nil {
347-
return fmt.Errorf("failed writing %s to GITHUB_ENV file: %w", key, err)
347+
logger.LogWarn("startup", "Skipping GITHUB_ENV TLS trust export: write failed for %s (%s): %v", githubEnvPath, key, err)
348+
return nil
348349
}
349350
}
350351

internal/cmd/proxy_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -428,7 +428,7 @@ func TestConfigureTLSTrustEnvironment(t *testing.T) {
428428
}
429429
})
430430

431-
t.Run("skips GITHUB_ENV append when env var is unset", func(t *testing.T) {
431+
t.Run("skips GITHUB_ENV append when env var is unset or empty", func(t *testing.T) {
432432
t.Setenv("GITHUB_ENV", "")
433433
require.NoError(t, configureTLSTrustEnvironment(caPath))
434434
})
@@ -452,6 +452,14 @@ func TestConfigureTLSTrustEnvironment(t *testing.T) {
452452
}
453453
})
454454

455+
t.Run("treats GITHUB_ENV write failures as best-effort", func(t *testing.T) {
456+
if _, err := os.Stat("/dev/full"); err != nil {
457+
t.Skip("/dev/full not available on this platform")
458+
}
459+
t.Setenv("GITHUB_ENV", "/dev/full")
460+
require.NoError(t, configureTLSTrustEnvironment(caPath))
461+
})
462+
455463
t.Run("rejects CA cert path with newline", func(t *testing.T) {
456464
err := configureTLSTrustEnvironment("/tmp/ca.crt\nMALICIOUS=1")
457465
require.Error(t, err)

0 commit comments

Comments
 (0)