Compare commits

..
Author SHA1 Message Date
BaaRai 69831f5cdf fix(security): apply New options so the secret scrub actually runs
New accepted opts ...Option but never called them, so scrubSecrets stayed
false in the binary and ScrubSecretEnv was unreachable: the protection merged
in #136 never engaged. The unit test passed because it called the scrub
function directly instead of going through New.

Apply the options in the constructor and add a test that builds a Server
through New with WithSecretScrub and asserts the secrets are gone. Verified to
fail without the loop.
2026-11-03 23:47:49 +01:00
3 changed files with 33 additions and 3 deletions
+1 -3
View File
@@ -31,9 +31,7 @@ while [ $# -gt 0 ]; do
*) echo "unknown option: $1" >&2; usage >&2; exit 2 ;;
esac
done
# 0* rejects 0, which would make prune delete the dump this run just wrote, and
# leading zeros, which $((KEEP + 1)) would read as octal.
case "$KEEP" in (*[!0-9]*|''|0*) echo "--keep must be a positive integer" >&2; exit 2 ;; esac
case "$KEEP" in (*[!0-9]*|'') echo "--keep must be a positive integer" >&2; exit 2 ;; esac
# Compose reads .env for credentials; source it for the dump command too.
if [ -f .env ]; then
+29
View File
@@ -1,6 +1,8 @@
package server
import (
"context"
"os"
"os/exec"
"strings"
"testing"
@@ -34,3 +36,30 @@ func TestScrubSecretEnvHidesSecretsFromChildren(t *testing.T) {
t.Errorf("non-secret variable did not reach the child process")
}
}
// New must apply the options it receives. An earlier version accepted
// ...Option and never called them, so the binary kept the secrets in the
// environment while the unit test above still passed.
func TestNewAppliesSecretScrubOption(t *testing.T) {
// t.Setenv restores both after the test: the scrub unsets them for the rest
// of this process, and later tests still need the DSN.
if dsn := os.Getenv("ARTEX_PG_DSN"); dsn != "" {
t.Setenv("ARTEX_PG_DSN", dsn)
}
t.Setenv("ANTHROPIC_API_KEY", "scrub-probe")
t.Setenv("OPENAI_API_KEY", "scrub-probe")
m, err := NewManager(t.TempDir(), "")
if err != nil {
t.Skipf("no manager: %v", err)
}
t.Cleanup(func() { m.Close() })
ctx, cancel := context.WithCancel(context.Background())
cancel()
_ = New(ctx, m, t.TempDir(), t.TempDir(), t.TempDir(), WithSecretScrub())
if os.Getenv("ANTHROPIC_API_KEY") != "" || os.Getenv("OPENAI_API_KEY") != "" {
t.Fatal("New did not apply WithSecretScrub: the secrets are still in the environment")
}
}
+3
View File
@@ -156,6 +156,9 @@ func New(ctx context.Context, m *Manager, skillDir string, dataDir string, keyDi
profChatAgents: map[int64]*agent.ChatAgent{},
provByProfile: map[int64]*provEntry{}, llmHealth: newLLMHealthRegistry(m.pg),
taskAgents: map[string]*taskAgentBundle{}, archiveWake: make(chan struct{}, 1)}
for _, o := range opts {
o(s)
}
s.initSetupToken()
s.initSideQuestions()
// Circuit-breaker thresholds/cooldowns are hot parameters on the failure path; push the global retry policy to the Registry once at startup,