diff --git a/harness.toml.example b/harness.toml.example index 93e9b0f..c06d412 100644 --- a/harness.toml.example +++ b/harness.toml.example @@ -1,8 +1,9 @@ # MOPAC harness configuration, v0 (2026-08-28). # # Copy to harness.toml and adjust. harness.toml is gitignored: secrets never -# live in this file, only refs (env:NAME | file:PATH | literal:VALUE; the -# bw: bitwarden ref lands with the key wrapper in build phase 3). +# live in this file, only refs (env:NAME | file:PATH | literal:VALUE | +# mpk:PLACEHOLDER via [keyproxy]; the bw: bitwarden ref lands with the key +# wrapper in build phase 3). # Vertical / stack identity for this harness instance. vertical = "demo" @@ -15,6 +16,10 @@ report_dir = "reports" [loop] # Bounded turn: max LLM round trips per task (tool calls included). max_rounds = 8 +# `harness loop` daemon: Redmine scan interval and state location +# (append-only loop.jsonl, dedup by issue id + updated_on). +poll_interval_secs = 120 +state_dir = "state/loop" [redmine] url = "https://rm.example.org" @@ -27,12 +32,37 @@ scope_query = "project=mopac&status_id=released&limit=25" class_field = "Class" default_class = "primary" +# `harness loop` status transitions: after a REPORT is noted back on the +# issue, an issue whose CURRENT status matches a key here is moved to the +# value. Statuses are names (resolved via /issue_statuses.json); an empty +# map leaves status alone. +[redmine.status_map] +"In Progress" = "Done" + [litellm] base_url = "http://192.168.3.78:4001" key_ref = "env:HARNESS_LITELLM_KEY" timeout_secs = 120 max_retries = 2 +# KEYPROXY (optional): resolve `mpk:` key refs through the ukrrs/mopac-keyproxy +# hop (POST /v1/resolve, bearer auth). Without this section, env:/file:/ +# literal: refs keep working as-is — the loop runs with or without keyproxy up. +# [keyproxy] +# url = "http://127.0.0.1:8082" +# token_ref = "env:HARNESS_KEYPROXY_TOKEN" # local ref only (no mpk: recursion) +# cache_ttl_secs = 60 + +# GITEA (optional, off by default): `harness loop` commits each REPORT file +# to this repo right after writing it (contents API, create-or-update). +# [gitea] +# url = "https://git.example.org" +# key_ref = "env:HARNESS_GITEA_KEY" +# owner = "ukrrs" +# repo = "MOPAC-reports" +# branch = "main" # empty = repo default branch +# commit_reports = false + # MODEL ROUTING v0 (static, config-only, no heuristics): [models] is the # tier map (tier alias -> concrete proxy model); [models.classes] maps task # classes to tiers. Requests go out with the CONCRETE model name resolved diff --git a/internal/config/config.go b/internal/config/config.go index 22719e9..9a0ac54 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -2,6 +2,7 @@ package config import ( "fmt" + "strings" ) // Config is the full harness.toml surface. Defaults live in Default(); @@ -17,10 +18,16 @@ type Config struct { Bash BashConfig Demo DemoConfig Events EventsConfig + KeyProxy KeyProxyConfig + Gitea GiteaConfig } +// LoopConfig is both the per-turn bound and the `harness loop` daemon +// cadence + state location. type LoopConfig struct { - MaxRounds int + MaxRounds int + PollIntervalSecs int // loop: Redmine scan interval (default 120) + StateDir string // loop: append-only loop.jsonl + dedup index } type RedmineConfig struct { @@ -31,6 +38,10 @@ type RedmineConfig struct { ClassField string DefaultClass string Limit int + // StatusMap: issue status name -> status name to set after a REPORT + // lands (loop path only; e.g. [redmine.status_map] "In Progress" = + // "Done"). Empty map = leave status alone. + StatusMap map[string]string } type LiteLLMConfig struct { @@ -65,6 +76,26 @@ type DemoConfig struct { Class string } +// KeyProxyConfig is the ukrrs/mopac-keyproxy resolve hop: mpk: key refs +// POST here. All hosts/paths live in this config, never in code. +type KeyProxyConfig struct { + URL string // hop base url (e.g. http://127.0.0.1:8082) + TokenRef string // local ref (env:/file:/literal:) holding the bearer token + CacheTTLSecs int // in-memory mpk resolution cache (default 60) +} + +// GiteaConfig is the optional REPORT-commit step (off by default). When +// commit_reports is true the loop commits each REPORT file to the +// configured repo right after writing it. +type GiteaConfig struct { + URL string + KeyRef string + Owner string + Repo string + Branch string + CommitReports bool +} + // EventsConfig is the `harness events` webhook receiver surface. type EventsConfig struct { Listen string // bind address (publish via docker -p) @@ -87,11 +118,16 @@ func Default() *Config { return &Config{ WorkRoot: ".", ReportDir: "reports", - Loop: LoopConfig{MaxRounds: 8}, + Loop: LoopConfig{ + MaxRounds: 8, + PollIntervalSecs: 120, + StateDir: "state/loop", + }, Redmine: RedmineConfig{ ClassField: "Class", DefaultClass: "primary", Limit: 50, + StatusMap: map[string]string{}, }, LiteLLM: LiteLLMConfig{TimeoutSecs: 120, MaxRetries: 2}, Models: ModelsConfig{ @@ -110,6 +146,7 @@ func Default() *Config { SecretHeader: "X-Discourse-Webhook-Secret", }, }, + KeyProxy: KeyProxyConfig{CacheTTLSecs: 60}, Demo: DemoConfig{ ID: "demo-1", Subject: "MVP demo: GLM self-description", @@ -132,6 +169,12 @@ func (c *Config) apply(doc TOMLDoc) error { if v, ok := doc.Table("loop").Int("max_rounds"); ok { c.Loop.MaxRounds = int(v) } + if v, ok := doc.Table("loop").Int("poll_interval_secs"); ok { + c.Loop.PollIntervalSecs = int(v) + } + if v, ok := doc.Table("loop").String("state_dir"); ok { + c.Loop.StateDir = v + } rm := doc.Table("redmine") if v, ok := rm.String("url"); ok { @@ -155,6 +198,14 @@ func (c *Config) apply(doc TOMLDoc) error { if v, ok := rm.Int("limit"); ok { c.Redmine.Limit = int(v) } + for _, k := range doc.Table("redmine", "status_map").Keys() { + if v, ok := doc.Table("redmine", "status_map").String(k); ok { + if c.Redmine.StatusMap == nil { + c.Redmine.StatusMap = map[string]string{} + } + c.Redmine.StatusMap[k] = v + } + } lt := doc.Table("litellm") if v, ok := lt.String("base_url"); ok { @@ -230,6 +281,37 @@ func (c *Config) apply(doc TOMLDoc) error { c.Demo.Class = v } + kp := doc.Table("keyproxy") + if v, ok := kp.String("url"); ok { + c.KeyProxy.URL = v + } + if v, ok := kp.String("token_ref"); ok { + c.KeyProxy.TokenRef = v + } + if v, ok := kp.Int("cache_ttl_secs"); ok { + c.KeyProxy.CacheTTLSecs = int(v) + } + + gt := doc.Table("gitea") + if v, ok := gt.String("url"); ok { + c.Gitea.URL = v + } + if v, ok := gt.String("key_ref"); ok { + c.Gitea.KeyRef = v + } + if v, ok := gt.String("owner"); ok { + c.Gitea.Owner = v + } + if v, ok := gt.String("repo"); ok { + c.Gitea.Repo = v + } + if v, ok := gt.String("branch"); ok { + c.Gitea.Branch = v + } + if v, ok := gt.Bool("commit_reports"); ok { + c.Gitea.CommitReports = v + } + ev := doc.Table("events") if v, ok := ev.String("listen"); ok { c.Events.Listen = v @@ -327,5 +409,41 @@ func (c *Config) Validate() error { if c.Events.Listen == "" || c.Events.StateDir == "" { return fmt.Errorf("[events]: listen and state_dir must not be empty") } + + if c.Loop.PollIntervalSecs < 1 { + return fmt.Errorf("[loop]: poll_interval_secs must be >= 1") + } + if c.Loop.StateDir == "" { + return fmt.Errorf("[loop]: state_dir must not be empty") + } + + // Keyproxy is optional; if any piece is set, url + token_ref must be. + if c.KeyProxy.URL != "" || c.KeyProxy.TokenRef != "" { + if c.KeyProxy.URL == "" { + return fmt.Errorf("[keyproxy]: url is required when keyproxy is configured") + } + if c.KeyProxy.TokenRef == "" { + return fmt.Errorf("[keyproxy]: token_ref is required when keyproxy is configured (the bearer token for /v1/resolve)") + } + if err := CheckKeyRef(c.KeyProxy.TokenRef); err != nil { + return fmt.Errorf("[keyproxy]: %w", err) + } + if strings.HasPrefix(c.KeyProxy.TokenRef, "mpk:") { + return fmt.Errorf("[keyproxy]: token_ref must be a local ref (env:/file:/literal:), not mpk:") + } + if c.KeyProxy.CacheTTLSecs < 1 { + return fmt.Errorf("[keyproxy]: bad cache_ttl_secs") + } + } + + // Gitea REPORT commit is optional; when on, the minimum set must be set. + if c.Gitea.CommitReports { + if c.Gitea.URL == "" || c.Gitea.KeyRef == "" || c.Gitea.Owner == "" || c.Gitea.Repo == "" { + return fmt.Errorf("[gitea]: url, key_ref, owner and repo are required when commit_reports = true") + } + if err := CheckKeyRef(c.Gitea.KeyRef); err != nil { + return fmt.Errorf("[gitea]: %w", err) + } + } return nil } diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 2a8da49..f3f03a9 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -123,7 +123,7 @@ func TestLoadValidationErrors(t *testing.T) { { name: "bare key value", mut: func(s string) string { return strings.Replace(s, `key_ref = "env:HARNESS_TEST_LLM_KEY"`, `key_ref = "sk-or-whatever"`, 1) }, - want: "env:, file:, literal:, or bw:", + want: "env:, file:, literal:, mpk:, or bw:", }, { name: "bw ref not implemented", @@ -289,3 +289,144 @@ secret_ref = "HARNESS_GITEA_HOOK" t.Errorf("error leaks ref value: %v", err) } } + +func TestLoopAndKeyProxyAndGiteaParsed(t *testing.T) { + cfg, err := Load(writeTemp(t, ` +vertical = "demo" +[litellm] +base_url = "http://x:4001" +key_ref = "mpk:mpk-litellm" +[models] +mopac-primary = "glm-5.3" + +[loop] +max_rounds = 6 +poll_interval_secs = 45 +state_dir = "st/loop" + +[redmine] +url = "https://rm.test" +key_ref = "mpk:redmine" +scope_query = "project=x" + +[redmine.status_map] +"In Progress" = "Done" +New = "In Progress" + +[keyproxy] +url = "http://127.0.0.1:8082" +token_ref = "literal:kp-token" +cache_ttl_secs = 30 + +[gitea] +url = "https://git.test" +key_ref = "mpk:gitea" +owner = "ukrrs" +repo = "MOPAC" +branch = "reports" +commit_reports = true +`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Loop.PollIntervalSecs != 45 || cfg.Loop.StateDir != "st/loop" || cfg.Loop.MaxRounds != 6 { + t.Errorf("loop wrong: %+v", cfg.Loop) + } + if cfg.Redmine.StatusMap["In Progress"] != "Done" || cfg.Redmine.StatusMap["New"] != "In Progress" { + t.Errorf("status map wrong: %+v", cfg.Redmine.StatusMap) + } + if cfg.KeyProxy.URL != "http://127.0.0.1:8082" || cfg.KeyProxy.TokenRef != "literal:kp-token" || cfg.KeyProxy.CacheTTLSecs != 30 { + t.Errorf("keyproxy wrong: %+v", cfg.KeyProxy) + } + if !cfg.Gitea.CommitReports || cfg.Gitea.Owner != "ukrrs" || cfg.Gitea.Repo != "MOPAC" || cfg.Gitea.Branch != "reports" { + t.Errorf("gitea wrong: %+v", cfg.Gitea) + } +} + +func TestLoopDefaults(t *testing.T) { + cfg, err := Load(writeTemp(t, ` +vertical = "demo" +[litellm] +base_url = "http://x:4001" +key_ref = "literal:k" +[models] +mopac-primary = "glm-5.3" +`)) + if err != nil { + t.Fatalf("Load: %v", err) + } + if cfg.Loop.PollIntervalSecs != 120 { + t.Errorf("default poll_interval_secs = %d, want 120", cfg.Loop.PollIntervalSecs) + } + if cfg.Loop.StateDir != "state/loop" { + t.Errorf("default loop state_dir = %q", cfg.Loop.StateDir) + } + if cfg.KeyProxy.URL != "" || cfg.Gitea.CommitReports { + t.Errorf("keyproxy/gitea must default off: %+v %+v", cfg.KeyProxy, cfg.Gitea) + } +} + +func TestKeyProxyValidation(t *testing.T) { + cases := []struct{ name, extra, want string }{ + { + name: "token without url", + extra: "[keyproxy]\ntoken_ref = \"literal:t\"\n", + want: "[keyproxy]: url is required", + }, + { + name: "url without token", + extra: "[keyproxy]\nurl = \"http://127.0.0.1:8082\"\n", + want: "[keyproxy]: token_ref is required", + }, + { + name: "mpk token ref (recursion)", + extra: "[keyproxy]\nurl = \"http://127.0.0.1:8082\"\ntoken_ref = \"mpk:self\"\n", + want: "must be a local ref", + }, + { + name: "gitea on but incomplete", + extra: "[gitea]\ncommit_reports = true\nurl = \"https://git.test\"\n", + want: "[gitea]: url, key_ref, owner and repo are required", + }, + { + name: "loop poll too small", + extra: "[loop]\npoll_interval_secs = 0\n", + want: "[loop]: poll_interval_secs must be >= 1", + }, + } + base := ` +vertical = "demo" +[litellm] +base_url = "http://x:4001" +key_ref = "literal:k" +[models] +mopac-primary = "glm-5.3" +` + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := Load(writeTemp(t, base+tc.extra)) + if err == nil { + t.Fatalf("expected error containing %q", tc.want) + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error %q does not contain %q", err, tc.want) + } + }) + } +} + +func TestCheckKeyRefMPK(t *testing.T) { + for _, ref := range []string{"mpk:mpk-redmine", "mpk:redmine"} { + if err := CheckKeyRef(ref); err != nil { + t.Errorf("CheckKeyRef(%q) = %v, want nil", ref, err) + } + } + if err := CheckKeyRef("mpk:"); err == nil { + t.Errorf("CheckKeyRef(\"mpk:\") should fail") + } + // The local resolver must point mpk: users at the configured resolver. + _, err := ResolveKeyRef("mpk:redmine") + if err == nil || !strings.Contains(err.Error(), "[keyproxy]") { + t.Errorf("local ResolveKeyRef on mpk: should name [keyproxy], got %v", err) + } +} diff --git a/internal/config/keys.go b/internal/config/keys.go index c084373..4996c25 100644 --- a/internal/config/keys.go +++ b/internal/config/keys.go @@ -1,9 +1,15 @@ package config import ( + "context" + "encoding/json" "fmt" + "io" + "net/http" "os" "strings" + "sync" + "time" ) // Secret key references. Values are never logged and never stored by the @@ -14,6 +20,10 @@ import ( // env:NAME environment variable (must be set and non-empty) // file:PATH file whose trimmed contents are the key // literal:VALUE inline key (last resort; still never logged) +// mpk:REF keyproxy placeholder (ukrrs/mopac-keyproxy); resolved via +// POST /v1/resolve. A `mpk:` prefix missing its own +// `mpk-` prefix is auto-prefixed ("mpk:redmine" == +// "mpk:mpk-redmine"). Needs [keyproxy] config (KeyResolver). // bw:REF bitwarden item (reserved; lands with the bw wrapper, phase 3) func CheckKeyRef(ref string) error { switch { @@ -31,15 +41,21 @@ func CheckKeyRef(ref string) error { if len(ref) <= 8 { return fmt.Errorf("key ref: literal: value is empty") } + case strings.HasPrefix(ref, "mpk:"): + if len(strings.TrimSpace(ref[4:])) == 0 { + return fmt.Errorf("key ref %q: mpk: needs a placeholder name", redact(ref)) + } case strings.HasPrefix(ref, "bw:"): return fmt.Errorf("key ref bw: not implemented yet (bitwarden wrapper lands in build phase 3)") default: - return fmt.Errorf("key ref must use env:, file:, literal:, or bw: prefixes (unrecognized ref redacted)") + return fmt.Errorf("key ref must use env:, file:, literal:, mpk:, or bw: prefixes (unrecognized ref redacted)") } return nil } -// ResolveKeyRef resolves a key reference to its value. Never log the result. +// ResolveKeyRef resolves a local key reference (env:, file:, literal:) to its +// value. mpk: refs need the keyproxy hop and a context; use KeyResolver. +// Never log the result. func ResolveKeyRef(ref string) (string, error) { if err := CheckKeyRef(ref); err != nil { return "", err @@ -63,13 +79,135 @@ func ResolveKeyRef(ref string) (string, error) { return v, nil case strings.HasPrefix(ref, "literal:"): return ref[8:], nil + case strings.HasPrefix(ref, "mpk:"): + return "", fmt.Errorf("key ref mpk: needs the [keyproxy] resolver (KeyResolver), not the local resolver") } return "", fmt.Errorf("unreachable key ref branch") } +// KeyResolver resolves key refs, mpk: included, against the configured +// keyproxy hop ([keyproxy] in harness.toml). mpk resolutions are cached in +// memory for a short TTL so the poll loop does not hammer the proxy; the +// material itself never leaves memory. With no [keyproxy] config, env:/file:/ +// literal: refs still resolve — the harness runs with or without keyproxy up. +type KeyResolver struct { + cfg KeyProxyConfig + http *http.Client + ttl time.Duration + + mu sync.Mutex + cache map[string]mpkEntry +} + +type mpkEntry struct { + value string + expires time.Time +} + +// NewKeyResolver builds a resolver from the config's [keyproxy] section. +func NewKeyResolver(cfg *Config) *KeyResolver { + ttl := time.Duration(cfg.KeyProxy.CacheTTLSecs) * time.Second + if ttl <= 0 { + ttl = 60 * time.Second + } + return &KeyResolver{ + cfg: cfg.KeyProxy, + http: &http.Client{Timeout: 15 * time.Second}, + ttl: ttl, + cache: make(map[string]mpkEntry), + } +} + +// Resolve resolves any supported key ref. Never log the result. +func (r *KeyResolver) Resolve(ctx context.Context, ref string) (string, error) { + if strings.HasPrefix(ref, "mpk:") { + return r.resolveMPK(ctx, ref[4:]) + } + return ResolveKeyRef(ref) +} + +// resolveMPK POSTs {"ref":"mpk-"} to the keyproxy /v1/resolve hop with +// the bearer token from [keyproxy] token_ref (itself a local ref, resolved +// once and cached for the process lifetime). +func (r *KeyResolver) resolveMPK(ctx context.Context, name string) (string, error) { + name = strings.TrimSpace(name) + if !strings.HasPrefix(name, "mpk-") { + name = "mpk-" + name + } + if r.cfg.URL == "" { + return "", fmt.Errorf("key ref mpk: no [keyproxy] url configured (fall back to env:/file: refs, or configure the hop)") + } + + r.mu.Lock() + if e, ok := r.cache[name]; ok && time.Now().Before(e.expires) { + r.mu.Unlock() + return e.value, nil + } + r.mu.Unlock() + + token, err := ResolveKeyRef(r.cfg.TokenRef) + if err != nil { + return "", fmt.Errorf("keyproxy token (%s): %w", redact(r.cfg.TokenRef), err) + } + + body, err := json.Marshal(map[string]string{"ref": name}) + if err != nil { + return "", err + } + url := strings.TrimSuffix(r.cfg.URL, "/") + "/v1/resolve" + req, err := http.NewRequestWithContext(ctx, http.MethodPost, url, strings.NewReader(string(body))) + if err != nil { + return "", fmt.Errorf("keyproxy request: %w", err) + } + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Authorization", "Bearer "+token) + resp, err := r.http.Do(req) + if err != nil { + return "", fmt.Errorf("keyproxy %s: %w", "resolve", redactErr(err)) + } + defer resp.Body.Close() + raw, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + if resp.StatusCode < 200 || resp.StatusCode >= 300 { + return "", fmt.Errorf("keyproxy resolve: HTTP %d: %s", resp.StatusCode, truncateErrBody(string(raw))) + } + var out struct { + Value string `json:"value"` + } + if err := json.Unmarshal(raw, &out); err != nil { + return "", fmt.Errorf("keyproxy decode: %w", err) + } + if out.Value == "" { + return "", fmt.Errorf("keyproxy resolve: empty value for %s", name) + } + + r.mu.Lock() + r.cache[name] = mpkEntry{value: out.Value, expires: time.Now().Add(r.ttl)} + r.mu.Unlock() + return out.Value, nil +} + +// redactErr strips any URL query or userinfo from transport errors so a +// misconfigured hop cannot echo the token back into logs. +func redactErr(err error) error { + msg := err.Error() + if i := strings.Index(msg, "?"); i >= 0 { + msg = msg[:i] + "?..." + } + return fmt.Errorf("%s", msg) +} + +func truncateErrBody(s string) string { + s = strings.TrimSpace(s) + if len(s) > 200 { + s = s[:200] + "..." + } + return s +} + // redact masks everything after a recognized prefix so malformed refs can // be reported without echoing a possibly-pasted secret. Refs without a -// recognizable prefix are never echoed at all. +// recognizable prefix are never echoed at all. mpk: refs are placeholders +// by construction, but they are masked too for uniformity. func redact(ref string) string { if i := strings.IndexByte(ref, ':'); i >= 0 && len(ref) > i+1 { return ref[:i+1] + "****" diff --git a/internal/config/keys_test.go b/internal/config/keys_test.go new file mode 100644 index 0000000..2e26fe4 --- /dev/null +++ b/internal/config/keys_test.go @@ -0,0 +1,152 @@ +package config + +import ( + "context" + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "sync/atomic" + "testing" +) + +// fakeKeyProxy mimics ukrrs/mopac-keyproxy POST /v1/resolve: bearer auth, +// {"ref":"mpk-"} in, {"value":...} out. +type fakeKeyProxy struct { + t *testing.T + token string + resolve func(ref string) (string, int) + hits atomic.Int64 + srv *httptest.Server +} + +func newFakeKeyProxy(t *testing.T, token string) *fakeKeyProxy { + t.Helper() + f := &fakeKeyProxy{t: t, token: token} + f.resolve = func(ref string) (string, int) { return "value-for-" + ref, 200 } + f.srv = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/v1/resolve" { + http.NotFound(w, r) + return + } + if got := r.Header.Get("Authorization"); got != "Bearer "+f.token { + http.Error(w, `{"error":"unauthorized"}`, http.StatusUnauthorized) + return + } + raw, _ := io.ReadAll(io.LimitReader(r.Body, 1<<20)) + var body struct { + Ref string `json:"ref"` + } + if err := json.Unmarshal(raw, &body); err != nil || body.Ref == "" { + http.Error(w, `{"error":"bad_request"}`, http.StatusBadRequest) + return + } + f.hits.Add(1) + value, code := f.resolve(body.Ref) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(code) + w.Write([]byte(`{"value":"` + value + `"}`)) + })) + t.Cleanup(f.srv.Close) + return f +} + +func TestKeyResolverMPK(t *testing.T) { + kp := newFakeKeyProxy(t, "kp-token") + cfg := Default() + cfg.KeyProxy.URL = kp.srv.URL + cfg.KeyProxy.TokenRef = "literal:kp-token" + r := NewKeyResolver(cfg) + + // Auto-prefix: mpk:redmine -> ref "mpk-redmine" on the wire. + v, err := r.Resolve(context.Background(), "mpk:redmine") + if err != nil { + t.Fatalf("Resolve: %v", err) + } + if v != "value-for-mpk-redmine" { + t.Errorf("value = %q", v) + } + // Explicit full form resolves identically (cache hit, no new call). + v, err = r.Resolve(context.Background(), "mpk:mpk-redmine") + if err != nil || v != "value-for-mpk-redmine" { + t.Errorf("full form = %q err=%v", v, err) + } + if n := kp.hits.Load(); n != 1 { + t.Errorf("keyproxy hits = %d, want 1 (cache must absorb the repeat)", n) + } + // A different ref is a fresh call. + if _, err := r.Resolve(context.Background(), "mpk:litellm"); err != nil { + t.Fatalf("Resolve litellm: %v", err) + } + if n := kp.hits.Load(); n != 2 { + t.Errorf("keyproxy hits = %d, want 2", n) + } +} + +func TestKeyResolverLocalRefsStillWork(t *testing.T) { + kp := newFakeKeyProxy(t, "kp-token") + cfg := Default() + cfg.KeyProxy.URL = kp.srv.URL + cfg.KeyProxy.TokenRef = "literal:kp-token" + r := NewKeyResolver(cfg) + + t.Setenv("HARNESS_KP_TEST", "envval") + for ref, want := range map[string]string{ + "env:HARNESS_KP_TEST": "envval", + "literal:abc": "abc", + } { + if v, err := r.Resolve(context.Background(), ref); err != nil || v != want { + t.Errorf("Resolve(%q) = %q err=%v, want %q", ref, v, err, want) + } + } + if n := kp.hits.Load(); n != 0 { + t.Errorf("local refs must not touch keyproxy, hits = %d", n) + } +} + +func TestKeyResolverErrors(t *testing.T) { + kp := newFakeKeyProxy(t, "kp-token") + + // No [keyproxy] url: clear error pointing at the config. + cfg := Default() + r := NewKeyResolver(cfg) + _, err := r.Resolve(context.Background(), "mpk:redmine") + if err == nil || !strings.Contains(err.Error(), "no [keyproxy] url") { + t.Errorf("no-url error = %v", err) + } + + // Bad token: 401 surfaces, the presented token never does. + cfg = Default() + cfg.KeyProxy.URL = kp.srv.URL + cfg.KeyProxy.TokenRef = "literal:wrong-token" + r = NewKeyResolver(cfg) + _, err = r.Resolve(context.Background(), "mpk:redmine") + if err == nil || !strings.Contains(err.Error(), "401") { + t.Errorf("401 error = %v", err) + } + if strings.Contains(err.Error(), "wrong-token") { + t.Errorf("error leaks token: %v", err) + } + + // Unknown ref: 404 with fixed body. + cfg = Default() + cfg.KeyProxy.URL = kp.srv.URL + cfg.KeyProxy.TokenRef = "literal:kp-token" + kp.resolve = func(ref string) (string, int) { return "", 404 } + r = NewKeyResolver(cfg) + _, err = r.Resolve(context.Background(), "mpk:nope") + if err == nil || !strings.Contains(err.Error(), "404") { + t.Errorf("404 error = %v", err) + } + + // Hop down: transport error, not a hang or panic. + cfg = Default() + cfg.KeyProxy.URL = "http://127.0.0.1:1" + cfg.KeyProxy.TokenRef = "literal:kp-token" + r = NewKeyResolver(cfg) + _, err = r.Resolve(context.Background(), "mpk:redmine") + if err == nil { + t.Errorf("dead hop should error") + } +} diff --git a/internal/config/toml.go b/internal/config/toml.go index ec8250f..ec29734 100644 --- a/internal/config/toml.go +++ b/internal/config/toml.go @@ -221,7 +221,15 @@ func parseKeyValue(m TOMLDoc, text string, no int) error { return fmt.Errorf("harness.toml:%d: expected key = value, got %q", no, text) } key := strings.TrimSpace(text[:eq]) - if !isBareKey(key) { + if strings.HasPrefix(key, `"`) { + // Quoted key (e.g. `"In Progress" = "Done"` in a status map): + // the key must be exactly one quoted string. + q, err := parseStringValue(key, no) + if err != nil { + return fmt.Errorf("harness.toml:%d: invalid quoted key: %w", no, err) + } + key = q + } else if !isBareKey(key) { return fmt.Errorf("harness.toml:%d: invalid key %q", no, key) } valStr := strings.TrimSpace(text[eq+1:])