Audit-driven follow-up to the Rust scrum review on the 3 untested
HIGH-risk packages. Both the audit (reports/scrum/risk-register.md)
and the scrum (tests/real-world/runs/scrum_mojxb5bw/) independently
flagged these files as the highest-leverage missing test coverage.
internal/shared/server_test.go — 8 test funcs
newListener: valid addr, invalid addr (non-numeric port, port
out of range, port-already-in-use surfacing as net.OpError).
Empty-addr-is-valid: documents the net.Listen quirk that "" binds
an OS-picked port — future readers don't need to relitigate.
HealthResponse marshal: JSON shape stable, round-trip clean.
/health handler reconstructed via httptest.Server: status 200,
Content-Type application/json, body fields stable.
RegisterRoutes callback: contract verified (callback is invoked
with a real chi.Router, mounted route reachable end-to-end).
Run bind-failure surface: synchronous error, not a goroutine swallow
— the contract Run depends on per the race-safe-startup comment.
internal/shared/config_test.go — 6 test funcs
DefaultConfig G0 port pinning: every binary's default bind locked
in (3110/3211-3216) so a refactor can't silently flip a port.
LoadConfig empty path: returns DefaultConfig, no error.
LoadConfig missing file: returns DefaultConfig, logs warn (the warn
line shows up in test output, captured-but-not-asserted).
LoadConfig valid TOML: partial overrides land, unspecified sections
keep defaults (TOML decoder leave-alone behavior).
LoadConfig invalid TOML: returns wrapped 'parse config' error.
LoadConfig unreadable file: skipped under root (root reads 0000);
captures the read-error wrap path for non-root contexts.
internal/storeclient/client_test.go — 14 test funcs
safeKey table-driven: plain segments, single slash, empty, trailing
slash, space (→ %20), apostrophe (→ %27), unicode (→ %C3%A9),
deep nesting. Locks URL-escape contract per scrum suggestion.
recordingServer helper backs Put/Get/Delete/List against
httptest.Server: verifies method, path, body bytes round-trip.
ErrKeyNotFound on 404 (errors.Is round-trip).
Non-OK status wraps body preview into the error chain.
Delete accepts both 200 and 204 (S3 vs compatible-store quirk).
List parses JSON shape and surfaces query-string prefix.
Context cancellation propagates through Put as context.Canceled.
internal/queryd/db_test.go — 5 test funcs (with subtests)
sqlEscape table-driven: 8 cases including empty, all-quotes,
nested apostrophes (the case from the scrum suggestion).
redactCreds table-driven: 6 cases — both keys, single keys,
empty, multi-occurrence, placeholder-collision (lossy but safe).
buildBootstrap statement order: INSTALL → LOAD → CREATE SECRET.
buildBootstrap endpoint schemes: http strips + USE_SSL false,
https keeps SSL true, no-scheme defaults SSL true (prod ambient).
buildBootstrap URL_STYLE: 'path' vs 'vhost' branch.
buildBootstrap escapes credential quotes: future SSO-token-with-
apostrophe doesn't break out of the SQL string literal — the
belt holds when the suspenders snap.
Real finding caught by my own test:
net.Listen("tcp", "") succeeds (OS-picked port) — captured as
TestNewListener_EmptyAddrIsValid so the quirk is documented.
Verified:
go test -short ./... — every internal/ package now has tests
(no more 'no test files' lines for shared/storeclient).
just verify — vet + test + 9 smokes green in 33s.
just proof contract — 53/0/1 green (no harness regression).
Closes:
R-002 internal/shared zero tests HIGH
R-003 internal/storeclient zero tests HIGH
R-008 queryd/db.go untested MED (sqlEscape, redactCreds,
CREATE SECRET formation)
Composite scrum score should move from 43 → ~46 / 60 — the three
HIGH/MED risks closed, internal/shared and internal/storeclient
become "tested + load-bearing" instead of "untested + load-bearing."
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
187 lines
5.5 KiB
Go
187 lines
5.5 KiB
Go
package queryd
|
|
|
|
import (
|
|
"strings"
|
|
"testing"
|
|
|
|
"git.agentview.dev/profit/golangLAKEHOUSE/internal/secrets"
|
|
"git.agentview.dev/profit/golangLAKEHOUSE/internal/shared"
|
|
)
|
|
|
|
// Closes R-008: db.go owns sqlEscape + redactCreds + buildBootstrap,
|
|
// none of which had tests. The first two are pure functions trivial
|
|
// to table-test; buildBootstrap is also pure (S3Config + creds → SQL
|
|
// strings) so we can exercise its endpoint-normalization branches
|
|
// without booting DuckDB.
|
|
|
|
func TestSqlEscape(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
in string
|
|
want string
|
|
}{
|
|
{"no quotes", "hello", "hello"},
|
|
{"single quote", "O'Reilly", "O''Reilly"},
|
|
{"double quote pair", "''", "''''"},
|
|
{"trailing quote", "foo'", "foo''"},
|
|
{"leading quote", "'foo", "''foo"},
|
|
{"empty string", "", ""},
|
|
{"only quotes", "'''", "''''''"},
|
|
{"mixed punctuation", "it's a 'test'", "it''s a ''test''"},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := sqlEscape(tc.in)
|
|
if got != tc.want {
|
|
t.Errorf("sqlEscape(%q) = %q, want %q", tc.in, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestRedactCreds(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
creds secrets.S3Credentials
|
|
msg string
|
|
want string
|
|
}{
|
|
{
|
|
"both keys redacted",
|
|
secrets.S3Credentials{AccessKeyID: "AKIATEST", SecretAccessKey: "topsecret"},
|
|
"failed: KEY_ID 'AKIATEST' SECRET 'topsecret'",
|
|
"failed: KEY_ID '[REDACTED-KEY]' SECRET '[REDACTED-SECRET]'",
|
|
},
|
|
{
|
|
"only access key present",
|
|
secrets.S3Credentials{AccessKeyID: "AKIATEST", SecretAccessKey: ""},
|
|
"echo: AKIATEST again",
|
|
"echo: [REDACTED-KEY] again",
|
|
},
|
|
{
|
|
"only secret present",
|
|
secrets.S3Credentials{AccessKeyID: "", SecretAccessKey: "mysecret"},
|
|
"echo: mysecret here",
|
|
"echo: [REDACTED-SECRET] here",
|
|
},
|
|
{
|
|
"empty creds = no change",
|
|
secrets.S3Credentials{},
|
|
"failed: nothing to scrub",
|
|
"failed: nothing to scrub",
|
|
},
|
|
{
|
|
"value appears multiple times",
|
|
secrets.S3Credentials{AccessKeyID: "AKIATEST"},
|
|
"AKIATEST failed because AKIATEST",
|
|
"[REDACTED-KEY] failed because [REDACTED-KEY]",
|
|
},
|
|
{
|
|
"key value collision with placeholder string is lossy but safe",
|
|
secrets.S3Credentials{AccessKeyID: "[REDACTED-KEY]"},
|
|
"loop: [REDACTED-KEY]",
|
|
"loop: [REDACTED-KEY]",
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
got := redactCreds(tc.msg, tc.creds)
|
|
if got != tc.want {
|
|
t.Errorf("redactCreds:\n msg=%q\n got=%q\n want=%q", tc.msg, got, tc.want)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestBuildBootstrap_StatementOrder(t *testing.T) {
|
|
stmts := buildBootstrap(
|
|
shared.S3Config{Endpoint: "http://localhost:9000", Region: "us-east-1", UsePathStyle: true},
|
|
secrets.S3Credentials{AccessKeyID: "key", SecretAccessKey: "secret"},
|
|
)
|
|
if len(stmts) != 3 {
|
|
t.Fatalf("want 3 statements, got %d: %v", len(stmts), stmts)
|
|
}
|
|
if stmts[0] != "INSTALL httpfs" {
|
|
t.Errorf("stmt[0] = %q, want INSTALL httpfs", stmts[0])
|
|
}
|
|
if stmts[1] != "LOAD httpfs" {
|
|
t.Errorf("stmt[1] = %q, want LOAD httpfs", stmts[1])
|
|
}
|
|
if !strings.HasPrefix(stmts[2], "CREATE OR REPLACE SECRET") {
|
|
t.Errorf("stmt[2] should start with CREATE OR REPLACE SECRET, got %q", stmts[2])
|
|
}
|
|
}
|
|
|
|
func TestBuildBootstrap_EndpointSchemes(t *testing.T) {
|
|
cases := []struct {
|
|
name string
|
|
endpoint string
|
|
wantHostInSQL string
|
|
wantUseSSLTrue bool
|
|
}{
|
|
{"http strips scheme, USE_SSL false",
|
|
"http://minio:9000", "minio:9000", false},
|
|
{"https keeps SSL true",
|
|
"https://s3.example.com", "s3.example.com", true},
|
|
{"no scheme defaults SSL true (ambient prod)",
|
|
"s3.amazonaws.com", "s3.amazonaws.com", true},
|
|
}
|
|
for _, tc := range cases {
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
stmts := buildBootstrap(
|
|
shared.S3Config{Endpoint: tc.endpoint, Region: "us-east-1"},
|
|
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
|
|
)
|
|
secret := stmts[2]
|
|
wantEndpointFrag := "ENDPOINT '" + tc.wantHostInSQL + "'"
|
|
if !strings.Contains(secret, wantEndpointFrag) {
|
|
t.Errorf("secret SQL missing %q\n got: %s", wantEndpointFrag, secret)
|
|
}
|
|
wantSSL := "USE_SSL false"
|
|
if tc.wantUseSSLTrue {
|
|
wantSSL = "USE_SSL true"
|
|
}
|
|
if !strings.Contains(secret, wantSSL) {
|
|
t.Errorf("secret SQL missing %q\n got: %s", wantSSL, secret)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestBuildBootstrap_URLStyle(t *testing.T) {
|
|
pathStmts := buildBootstrap(
|
|
shared.S3Config{Endpoint: "http://m:9000", UsePathStyle: true},
|
|
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
|
|
)
|
|
if !strings.Contains(pathStmts[2], "URL_STYLE 'path'") {
|
|
t.Errorf("UsePathStyle=true should produce URL_STYLE 'path'\n got: %s", pathStmts[2])
|
|
}
|
|
|
|
vhostStmts := buildBootstrap(
|
|
shared.S3Config{Endpoint: "https://m", UsePathStyle: false},
|
|
secrets.S3Credentials{AccessKeyID: "k", SecretAccessKey: "s"},
|
|
)
|
|
if !strings.Contains(vhostStmts[2], "URL_STYLE 'vhost'") {
|
|
t.Errorf("UsePathStyle=false should produce URL_STYLE 'vhost'\n got: %s", vhostStmts[2])
|
|
}
|
|
}
|
|
|
|
func TestBuildBootstrap_EscapesCredentialQuotes(t *testing.T) {
|
|
// Per the inline comment: "creds shouldn't contain ' but a future
|
|
// SSO token might." This is the test that asserts the belt holds
|
|
// when the suspenders snap.
|
|
stmts := buildBootstrap(
|
|
shared.S3Config{Endpoint: "https://m", Region: "us-east-1"},
|
|
secrets.S3Credentials{
|
|
AccessKeyID: "key'with'quotes",
|
|
SecretAccessKey: "secret",
|
|
},
|
|
)
|
|
secret := stmts[2]
|
|
// Escaped form: each ' became ''.
|
|
want := "KEY_ID 'key''with''quotes'"
|
|
if !strings.Contains(secret, want) {
|
|
t.Errorf("expected escaped key in SQL\n want fragment: %s\n got: %s", want, secret)
|
|
}
|
|
}
|