From 9de17b9289f8b7873c98f4869bff6a1bebf042d3 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:26 +0200 Subject: [PATCH 1/7] fix(transport): enforce the 16-MB response cap at the HTTP body read The soap decoders cap their input at soap.MaxResponseBytes, but the live path buffers the whole body in transport.doOnce first, so the guard was dead code. The cap now applies at the read; an oversized response fails immediately and is not retried. --- internal/transport/client.go | 9 +++++- internal/transport/client_test.go | 30 +++++++++++++++++++ .../get_server_information_request.xml | 0 ...et_server_information_response_success.xml | 0 4 files changed, 38 insertions(+), 1 deletion(-) rename testdata/{account => server}/get_server_information_request.xml (100%) rename testdata/{account => server}/get_server_information_response_success.xml (100%) diff --git a/internal/transport/client.go b/internal/transport/client.go index 3b4a70c..a479d0f 100644 --- a/internal/transport/client.go +++ b/internal/transport/client.go @@ -11,6 +11,7 @@ import ( "sync" "time" + "github.com/chmmou/kasapi-cli/internal/soap" "github.com/chmmou/kasapi-cli/internal/version" ) @@ -170,10 +171,16 @@ func (c *Client) doOnce(ctx context.Context, endpoint string, body []byte) ([]by } defer func() { _ = resp.Body.Close() }() - respBody, err := io.ReadAll(resp.Body) + // The soap decoders cap their input at soap.MaxResponseBytes, but by + // the time they run the whole body has already been buffered here — + // so the memory-exhaustion guard must be enforced at this read. + respBody, err := io.ReadAll(io.LimitReader(resp.Body, soap.MaxResponseBytes+1)) if err != nil { return nil, &retryableError{err: fmt.Errorf("transport: read body: %w", err)} } + if len(respBody) > soap.MaxResponseBytes { + return nil, fmt.Errorf("transport: response from %s exceeds %d bytes", endpoint, soap.MaxResponseBytes) + } if resp.StatusCode >= 500 { return nil, &retryableError{err: fmt.Errorf("transport: %s returned %s", endpoint, resp.Status)} diff --git a/internal/transport/client_test.go b/internal/transport/client_test.go index b9fedb5..af78b6c 100644 --- a/internal/transport/client_test.go +++ b/internal/transport/client_test.go @@ -388,3 +388,33 @@ func TestDoRespectsContextDeadlineDuringRequest(t *testing.T) { t.Fatal("expected error on cancelled context") } } + +// The 16-MB soap.MaxResponseBytes cap must bite at the transport read: +// the api/auth clients buffer the whole body here before the capped +// soap decoders ever run, so this is the only place the guard can +// actually prevent memory exhaustion. Oversize is not retryable. +func TestDoRejectsOversizedResponse(t *testing.T) { + var hits atomic.Int32 + chunk := bytes.Repeat([]byte("x"), 1<<20) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + hits.Add(1) + for i := 0; i < 17; i++ { + if _, err := w.Write(chunk); err != nil { + return + } + } + })) + defer srv.Close() + + c := newClient(srv, newFakeClock()) + _, err := c.Do(context.Background(), srv.URL, []byte(sampleEnvelope)) + if err == nil { + t.Fatal("Do: want oversize error, got nil") + } + if !strings.Contains(err.Error(), "exceeds") { + t.Errorf("err = %v, want a response-size error", err) + } + if hits.Load() != 1 { + t.Errorf("requests = %d, want 1 (oversize must not be retried)", hits.Load()) + } +} diff --git a/testdata/account/get_server_information_request.xml b/testdata/server/get_server_information_request.xml similarity index 100% rename from testdata/account/get_server_information_request.xml rename to testdata/server/get_server_information_request.xml diff --git a/testdata/account/get_server_information_response_success.xml b/testdata/server/get_server_information_response_success.xml similarity index 100% rename from testdata/account/get_server_information_response_success.xml rename to testdata/server/get_server_information_response_success.xml From d1f851045c8d9e33d3a9d603e22bdf0c5123cee9 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:26 +0200 Subject: [PATCH 2/7] fix(cli): render write success output through the --output pipeline Write commands previously printed their success line as plain text regardless of --output, breaking --output=json scripting. The line now renders via the shared Render seam wrapped in a writeResult: table output stays the bare line, json/yaml emit a {"message": ...} object. --- internal/cli/export_test.go | 6 +++++ internal/cli/run.go | 19 +++++++++++--- internal/cli/run_test.go | 49 +++++++++++++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 internal/cli/run_test.go diff --git a/internal/cli/export_test.go b/internal/cli/export_test.go index 4a735ad..b6ce12a 100644 --- a/internal/cli/export_test.go +++ b/internal/cli/export_test.go @@ -34,6 +34,12 @@ var RevokeSession = revokeSession // temp session.Store, mirroring the `config use-profile` pattern. var RunSessionsDelete = runSessionsDelete +// WriteResult mirrors the unexported writeResult wrapper runWriteE +// renders its success line through, so tests can pin the --output +// contract of write commands (table = bare line, json/yaml = message +// object) without a network dispatch. +type WriteResult = writeResult + // DatabaseDeleteConfirm and MailAccountDeleteConfirm expose the // package-private helpers that build the delete ConfirmAction for their // slices. Tests use them to pin the "permanently delete" loudness diff --git a/internal/cli/run.go b/internal/cli/run.go index 4113e35..5665136 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -2,7 +2,6 @@ package cli import ( "context" - "fmt" "io" "time" @@ -160,9 +159,23 @@ func runWriteE(opts *RootOptions, build func(args []string) (writeSpec, error)) if derr != nil { return APIError(derr, spec.action) } - if _, perr := fmt.Fprintln(out, result); perr != nil { - return UserError(perr, "render") + if rerr := Render(out, opts.Output, writeResult{Message: result}); rerr != nil { + return UserError(rerr, "render") } return nil } } + +// writeResult wraps the human success line a write dispatch returns so +// it renders through the same --output pipeline the read commands use: +// table output stays the bare line (no header row), json/yaml emit a +// {"message": ...} object that scripts can parse. +type writeResult struct { + Message string `json:"message" yaml:"message"` +} + +// TableHeaders implements Tabular; a success line needs no header row. +func (writeResult) TableHeaders() []string { return nil } + +// TableRows implements Tabular. +func (r writeResult) TableRows() [][]string { return [][]string{{r.Message}} } diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go new file mode 100644 index 0000000..996c2fe --- /dev/null +++ b/internal/cli/run_test.go @@ -0,0 +1,49 @@ +package cli_test + +import ( + "bytes" + "encoding/json" + "strings" + "testing" + + "github.com/chmmou/kasapi-cli/internal/cli" +) + +// runWriteE renders its success line through the writeResult wrapper so +// write commands honour --output like the read commands: table stays +// the bare human line, json/yaml wrap it in a message object scripts +// can parse. +func TestWriteResultRendersPerFormat(t *testing.T) { + t.Parallel() + r := cli.WriteResult{Message: "updated mailing list L"} + + var table bytes.Buffer + if err := cli.Render(&table, cli.FormatTable, r); err != nil { + t.Fatalf("Render table: %v", err) + } + if got := table.String(); got != "updated mailing list L\n" { + t.Errorf("table output = %q, want the bare success line", got) + } + + var jsonBuf bytes.Buffer + if err := cli.Render(&jsonBuf, cli.FormatJSON, r); err != nil { + t.Fatalf("Render json: %v", err) + } + var got struct { + Message string `json:"message"` + } + if err := json.Unmarshal(jsonBuf.Bytes(), &got); err != nil { + t.Fatalf("unmarshal: %v\n%s", err, jsonBuf.String()) + } + if got.Message != "updated mailing list L" { + t.Errorf("json message = %q, want the success line", got.Message) + } + + var yamlBuf bytes.Buffer + if err := cli.Render(&yamlBuf, cli.FormatYAML, r); err != nil { + t.Fatalf("Render yaml: %v", err) + } + if !strings.Contains(yamlBuf.String(), "message: updated mailing list L") { + t.Errorf("yaml output = %q, want a message field", yamlBuf.String()) + } +} From 8bdf72a044c4f8967e94fbd160710fc455aad326 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:26 +0200 Subject: [PATCH 3/7] fix(cli): elide multi-line and oversized values from audit records update_mailinglist --config-file / --subscriber blobs reached the stderr logfmt line, the --audit-log JSON sink, and the --dry-run preview verbatim; the list config can carry the list password in cleartext. RedactParams now replaces any multi-line or >256-byte value with an marker. --- internal/cli/audit.go | 26 ++++++++++++++++------ internal/cli/audit_test.go | 44 ++++++++++++++++++++++++++++++++------ internal/cli/mail_test.go | 7 ++++-- 3 files changed, 63 insertions(+), 14 deletions(-) diff --git a/internal/cli/audit.go b/internal/cli/audit.go index d1d90b8..8460574 100644 --- a/internal/cli/audit.go +++ b/internal/cli/audit.go @@ -66,6 +66,12 @@ var auditSecretParams = map[string]struct{}{ const auditRedacted = "" +// maxAuditValueLen caps how long a single parameter value may be before +// RedactParams elides it. Normal write parameters (names, hosts, Y/N +// toggles) are far shorter; only wholesale blobs like the mailing-list +// config exceed it. +const maxAuditValueLen = 256 + // redactParam reports whether the value of parameter key must be // redacted before it is logged. func redactParam(key string) bool { @@ -87,8 +93,12 @@ func redactParam(key string) bool { // RedactParams converts a KAS request/response parameter map into the // string map stored on AuditRecord.Fields, replacing every secret value -// (see redactParam) with auditRedacted. Non-string values are rendered -// with %v. A nil/empty map yields nil so the field is omitted. +// (see redactParam) with auditRedacted. Multi-line or oversized values +// (mailing-list config / subscriber blobs sent wholesale by +// update_mailinglist) are elided to a "" marker: the +// list config can carry the list password in cleartext, so the blob +// content must never reach either audit sink. Non-string values are +// rendered with %v. A nil/empty map yields nil so the field is omitted. func RedactParams(params map[string]any) map[string]string { if len(params) == 0 { return nil @@ -99,7 +109,11 @@ func RedactParams(params map[string]any) map[string]string { out[k] = auditRedacted continue } - out[k] = fmt.Sprintf("%v", v) + s := fmt.Sprintf("%v", v) + if strings.ContainsAny(s, "\n\r") || len(s) > maxAuditValueLen { + s = fmt.Sprintf("", len(s)) + } + out[k] = s } return out } @@ -137,9 +151,9 @@ func (r AuditRecord) logfmt() string { // whitespace, a quote, or '=' so the logfmt line stays unambiguous to // split on. Backslash and quote are escaped; a newline or carriage // return is escaped to the two-character \n / \r so a single field -// value can never split the record across physical lines (multi-line -// values reach here via e.g. update_mailinglist --subscriber / -// --config-file). +// value can never split the record across physical lines. RedactParams +// already elides multi-line blobs, so this escaping is defense-in-depth +// for values that reach Fields through another path. func quoteIfNeeded(v string) string { if v == "" { return `""` diff --git a/internal/cli/audit_test.go b/internal/cli/audit_test.go index 2c47a9b..50877ab 100644 --- a/internal/cli/audit_test.go +++ b/internal/cli/audit_test.go @@ -54,6 +54,37 @@ func TestRedactParams(t *testing.T) { } } +// Multi-line or oversized parameter values (the wholesale mailing-list +// config / subscriber blobs of update_mailinglist) must never reach the +// audit sinks verbatim: the list config can carry the list password in +// cleartext. +func TestRedactParamsElidesBlobs(t *testing.T) { + t.Parallel() + got := cli.RedactParams(map[string]any{ + "config": "line1\npassword secret123\n", + "subscriber": "a@x.de\rb@x.de", + "long": strings.Repeat("x", 300), + "comment": "short stays", + }) + if got["config"] != "" { + t.Errorf("config = %q, want ", got["config"]) + } + if got["subscriber"] != "" { + t.Errorf("subscriber = %q, want ", got["subscriber"]) + } + if got["long"] != "" { + t.Errorf("long = %q, want ", got["long"]) + } + if got["comment"] != "short stays" { + t.Errorf("comment = %q, want kept verbatim", got["comment"]) + } + for k, v := range got { + if strings.Contains(v, "secret123") { + t.Errorf("blob content leaked via %q = %q", k, v) + } + } +} + func TestOutcomeFor(t *testing.T) { t.Parallel() if got := cli.OutcomeFor(nil); got != "success" { @@ -108,11 +139,12 @@ func TestAuditRecordLogfmt(t *testing.T) { } } -// A field value containing a newline (e.g. update_mailinglist -// --subscriber a@x --subscriber b@x, or --config-file content) must not -// split the stderr audit record across physical lines: the embedded -// newline is escaped to the two-character \n inside a quoted value, so -// the record stays a single logfmt line. +// A field value containing a newline must not split the stderr audit +// record across physical lines: the embedded newline is escaped to the +// two-character \n inside a quoted value, so the record stays a single +// logfmt line. RedactParams elides multi-line blobs before they reach +// Fields, so the map is built directly here — the escaping is +// defense-in-depth for values arriving through another path. func TestAuditRecordLogfmtEscapesNewlines(t *testing.T) { t.Parallel() var stderr bytes.Buffer @@ -122,7 +154,7 @@ func TestAuditRecordLogfmtEscapesNewlines(t *testing.T) { Action: "update_mailinglist", Target: "announce-example-com", Outcome: "success", - Fields: cli.RedactParams(map[string]any{"subscriber": "a@x.de\nb@x.de"}), + Fields: map[string]string{"subscriber": "a@x.de\nb@x.de"}, } if err := cli.WriteAudit(&stderr, nil, rec); err != nil { t.Fatalf("WriteAudit: %v", err) diff --git a/internal/cli/mail_test.go b/internal/cli/mail_test.go index 4ee928a..ea3768a 100644 --- a/internal/cli/mail_test.go +++ b/internal/cli/mail_test.go @@ -570,7 +570,10 @@ func TestMailListsDestructiveRefuseNonTTY(t *testing.T) { // cobra Changed), --active maps to is_active Y/N, and --subscriber / // --restrict-post repeats join with a newline. --dry-run renders the // exact KAS params it would dispatch as JSON, so this asserts the -// assembly end to end without a network call. +// assembly end to end without a network call. Multi-line values are +// elided by RedactParams before they reach the preview, so the +// newline-join is pinned via the elided byte count (two 6-byte +// addresses + one separator byte = 13). func TestMailListsUpdateDryRunFieldAssembly(t *testing.T) { t.Parallel() cases := []struct { @@ -594,7 +597,7 @@ func TestMailListsUpdateDryRunFieldAssembly(t *testing.T) { { "subscriber repeats join with newline", []string{"mail", "lists", "update", "L", "--subscriber", "a@x.de", "--subscriber", "b@x.de"}, - map[string]string{"mailinglist_name": "L", "subscriber": "a@x.de\nb@x.de"}, + map[string]string{"mailinglist_name": "L", "subscriber": ""}, []string{"is_active"}, }, } From 22143e68346375bcce15fd852f63937fa9c5c1c6 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:38 +0200 Subject: [PATCH 4/7] fix(session): stop heartbeats from clobbering newer persisted tokens Heartbeat blindly re-saved its process's in-memory token with a fresh expiry, overwriting a newer token another process had persisted since. The new session.Store.Refresh extends an entry only while the on-disk token still matches; Save's doc no longer promises what it cannot hold. --- internal/auth/client_test.go | 41 ++++++++++++++++++++++++++ internal/auth/source.go | 8 +++-- internal/session/store.go | 45 ++++++++++++++++++++++++++--- internal/session/store_test.go | 53 ++++++++++++++++++++++++++++++++++ 4 files changed, 141 insertions(+), 6 deletions(-) diff --git a/internal/auth/client_test.go b/internal/auth/client_test.go index 3a62994..0f8208d 100644 --- a/internal/auth/client_test.go +++ b/internal/auth/client_test.go @@ -331,6 +331,47 @@ func TestSessionTokenSourceHeartbeatExtendsExpiry(t *testing.T) { } } +// A heartbeat must not clobber a newer token another process persisted +// after this process authenticated: the source's stale in-memory token +// no longer matches the on-disk entry, so the rolling-window refresh is +// skipped and the newer entry survives. +func TestSessionTokenSourceHeartbeatKeepsNewerPersistedToken(t *testing.T) { + body := loadFixture(t, "session/add_session_response_success.xml") + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write(body) + })) + defer srv.Close() + + tNow := time.Date(2026, 5, 3, 12, 0, 0, 0, time.UTC) + store := newStore(t, tNow) + src := auth.NewSessionTokenSource(newAuthClient(srv, "w0", "secret", soap.AuthPlain, auth.Options{})) + src.Store = store + src.Lifetime = time.Hour + src.UpdateLifetime = true + src.Now = func() time.Time { return tNow } + + if _, _, _, err := src.Credentials(context.Background()); err != nil { + t.Fatalf("Credentials: %v", err) + } + // Another process re-authenticates and persists a newer token. + newer := session.Entry{Token: "newer-token", ExpiresAt: tNow.Add(30 * time.Minute)} + if err := store.Save(t.Context(), "w0", newer); err != nil { + t.Fatalf("Save newer: %v", err) + } + + tNow = tNow.Add(15 * time.Minute) + store.Now = func() time.Time { return tNow } + src.Heartbeat(t.Context()) + + got, _ := store.Load(t.Context(), "w0") + if got == nil { + t.Fatal("expected the newer entry to survive the Heartbeat") + } + if got.Token != "newer-token" || !got.ExpiresAt.Equal(newer.ExpiresAt) { + t.Errorf("entry after Heartbeat = %+v, want the newer token kept", got) + } +} + func TestSessionTokenSourceAdoptsLifetimeFromCachedEntry(t *testing.T) { // Source created with no lifetime / update flags (e.g. a CLI run // without the KasAuth flags). It picks up a token persisted by an diff --git a/internal/auth/source.go b/internal/auth/source.go index 29323f1..0baf9ca 100644 --- a/internal/auth/source.go +++ b/internal/auth/source.go @@ -190,8 +190,12 @@ func (s *SessionTokenSource) Heartbeat(ctx context.Context) { LifetimeSeconds: int(s.lifetime() / time.Second), UpdateLifetime: true, } - if err := s.Store.Save(ctx, s.Client.Login, entry); err != nil { - s.logger().Warn("auth: session store heartbeat save failed; rolling window stays in-memory", "err", err) + // Refresh, not Save: another process may have persisted a newer + // token since this one authenticated; re-saving the stale token + // with a fresh expiry would clobber that update. Refresh only + // extends the entry while the on-disk token still matches. + if err := s.Store.Refresh(ctx, s.Client.Login, entry); err != nil { + s.logger().Warn("auth: session store heartbeat refresh failed; rolling window stays in-memory", "err", err) } } } diff --git a/internal/session/store.go b/internal/session/store.go index f6cf271..2989eb6 100644 --- a/internal/session/store.go +++ b/internal/session/store.go @@ -168,10 +168,12 @@ func (s *Store) Load(ctx context.Context, login string) (*Entry, error) { // ExpiresAt is zero, it is computed as Now+LifetimeSeconds (or // Now+DefaultLifetime when LifetimeSeconds is 0). // -// Save serialises with concurrent Load / Delete / Save calls (including -// from other kasapi-cli processes) via an advisory file lock at -// LockPath so a Heartbeat from one process cannot lose another -// process's update. ctx cancels the wait for the lock. +// Save serialises with concurrent Load / Delete / Save / Refresh calls +// (including from other kasapi-cli processes) via an advisory file lock +// at LockPath. Heartbeats must go through Refresh, not Save — Save +// replaces unconditionally and would clobber a newer token another +// process has persisted in the meantime. ctx cancels the wait for the +// lock. func (s *Store) Save(ctx context.Context, login string, e Entry) error { if login == "" { return errors.New("session: Save requires login") @@ -195,6 +197,41 @@ func (s *Store) Save(ctx context.Context, login string, e Entry) error { }) } +// Refresh persists e under login only while the stored entry still +// carries the same token as e.Token. It is the Heartbeat counterpart of +// Save: a heartbeat re-persists the token its process authenticated +// with plus a fresh expiry, so when another process has since saved a +// different (newer) token, writing the stale one back would clobber +// that update — the newer entry is left untouched instead. A missing +// file or entry is likewise left alone: there is nothing the stale +// token may extend. If ExpiresAt is zero it is computed as in Save. +// +// Refresh serialises with concurrent Load / Save / Delete calls via the +// advisory file lock at LockPath. ctx cancels the wait for the lock. +func (s *Store) Refresh(ctx context.Context, login string, e Entry) error { + if login == "" { + return errors.New("session: Refresh requires login") + } + if e.Token == "" { + return errors.New("session: Refresh requires token") + } + if e.ExpiresAt.IsZero() { + e.ExpiresAt = s.now().Add(s.lifetime(e)) + } + return s.withLock(ctx, func() error { + file, err := s.read() + if err != nil { + return err + } + cur, ok := file.Sessions[login] + if !ok || cur.Token != e.Token { + return nil + } + file.Sessions[login] = e + return s.write(file) + }) +} + // Delete removes the entry for login. Missing files and missing // entries are not errors. The file itself is removed when the last // entry is taken out so the on-disk state matches "no sessions". diff --git a/internal/session/store_test.go b/internal/session/store_test.go index 0e27f92..4ec71f1 100644 --- a/internal/session/store_test.go +++ b/internal/session/store_test.go @@ -272,3 +272,56 @@ func TestStoreReleasesLockAfterSave(t *testing.T) { } _ = ext.Unlock() } + +func TestRefreshExtendsMatchingToken(t *testing.T) { + now := time.Date(2026, 5, 3, 12, 0, 0, 0, time.UTC) + s := newStore(t, now) + if err := s.Save(t.Context(), "w0", session.Entry{Token: "tok", ExpiresAt: now.Add(time.Hour)}); err != nil { + t.Fatalf("Save: %v", err) + } + next := session.Entry{Token: "tok", ExpiresAt: now.Add(2 * time.Hour), LifetimeSeconds: 7200} + if err := s.Refresh(t.Context(), "w0", next); err != nil { + t.Fatalf("Refresh: %v", err) + } + got, err := s.Load(t.Context(), "w0") + if err != nil || got == nil { + t.Fatalf("Load: %v %v", got, err) + } + if !got.ExpiresAt.Equal(next.ExpiresAt) || got.LifetimeSeconds != 7200 { + t.Errorf("entry after Refresh = %+v, want extended expiry %v", got, next.ExpiresAt) + } +} + +// A heartbeat re-persists the token its process authenticated with; +// when another process has since saved a different (newer) token, +// Refresh must leave that newer entry untouched instead of clobbering +// it with the stale token. +func TestRefreshSkipsWhenTokenDiffers(t *testing.T) { + now := time.Date(2026, 5, 3, 12, 0, 0, 0, time.UTC) + s := newStore(t, now) + newer := session.Entry{Token: "newer", ExpiresAt: now.Add(30 * time.Minute)} + if err := s.Save(t.Context(), "w0", newer); err != nil { + t.Fatalf("Save: %v", err) + } + stale := session.Entry{Token: "stale", ExpiresAt: now.Add(2 * time.Hour)} + if err := s.Refresh(t.Context(), "w0", stale); err != nil { + t.Fatalf("Refresh: %v", err) + } + got, err := s.Load(t.Context(), "w0") + if err != nil || got == nil { + t.Fatalf("Load: %v %v", got, err) + } + if got.Token != "newer" || !got.ExpiresAt.Equal(newer.ExpiresAt) { + t.Errorf("entry after stale Refresh = %+v, want the newer entry kept", got) + } +} + +func TestRefreshMissingEntryIsNoop(t *testing.T) { + s := newStore(t, time.Now()) + if err := s.Refresh(t.Context(), "w0", session.Entry{Token: "tok"}); err != nil { + t.Errorf("Refresh on missing file: %v", err) + } + if got, _ := s.Load(t.Context(), "w0"); got != nil { + t.Errorf("Refresh created an entry: %+v", got) + } +} From 87ece43821e42cfe3a2d8ba833c6856786ddc848 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:38 +0200 Subject: [PATCH 5/7] test(mailinglist): fix dead fault-map key so the domain pin runs The add_mailinglist fault-map key carried a doubled mailinglist_ prefix and matched no fixture; AssertFaultFixtures skips unknown keys silently, so the mailinglist_domain_doesnt_exist code pin never ran. --- internal/mailinglist/write_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/internal/mailinglist/write_test.go b/internal/mailinglist/write_test.go index c6fe4c3..816d41b 100644 --- a/internal/mailinglist/write_test.go +++ b/internal/mailinglist/write_test.go @@ -148,18 +148,14 @@ func TestParamBuilders(t *testing.T) { // TestFaultFixturesDecodeToDocumentedCodes binds the captured // *_response_failed_*.xml fixtures to the KAS contract via the shared -// testutil.AssertFaultFixtures anchor. The -// add_mailinglist_..._mailinglist_mailinglist_domain_doesnt_exist -// sample is pinned because its filename duplicates the mailinglist_ -// prefix while the fault code does not — a code drift the curated map -// would catch. +// testutil.AssertFaultFixtures anchor. func TestFaultFixturesDecodeToDocumentedCodes(t *testing.T) { t.Parallel() testutil.AssertFaultFixtures(t, "mailinglist", map[string]string{ - "add_mailinglist_response_failed_missing_parameter.xml": "missing_parameter", - "add_mailinglist_response_failed_mailinglist_mailinglist_domain_doesnt_exist.xml": "mailinglist_domain_doesnt_exist", - "update_mailinglist_response_failed_nothing_to_do.xml": "nothing_to_do", - "update_mailinglist_response_failed_subscriber_email_syntax_incorrect.xml": "subscriber_email_syntax_incorrect", - "delete_mailinglist_response_failed_mailinglist_not_found.xml": "mailinglist_not_found", + "add_mailinglist_response_failed_missing_parameter.xml": "missing_parameter", + "add_mailinglist_response_failed_mailinglist_domain_doesnt_exist.xml": "mailinglist_domain_doesnt_exist", + "update_mailinglist_response_failed_nothing_to_do.xml": "nothing_to_do", + "update_mailinglist_response_failed_subscriber_email_syntax_incorrect.xml": "subscriber_email_syntax_incorrect", + "delete_mailinglist_response_failed_mailinglist_not_found.xml": "mailinglist_not_found", }) } From 7129c034f8593b18fd604cdf52f905b2a4ff93f2 Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:38 +0200 Subject: [PATCH 6/7] test(server,usage): move server fixtures home and add fault coverage get_server_information fixtures move from testdata/account/ to their own testdata/server/ per the one-subdir-per-module convention. server and usage gain the previously missing fault leg: a no_auth fault fixture per action (the captured auth fault is action-independent) pinned via testutil.AssertFaultFixtures. --- internal/server/server_test.go | 16 +++++++++++++--- internal/soap/soap_test.go | 2 +- internal/usage/usage_test.go | 12 ++++++++++++ ...erver_information_response_failed_no_auth.xml | 11 +++++++++++ .../usage/get_space_response_failed_no_auth.xml | 11 +++++++++++ .../get_space_usage_response_failed_no_auth.xml | 11 +++++++++++ .../get_traffic_response_failed_no_auth.xml | 11 +++++++++++ 7 files changed, 70 insertions(+), 4 deletions(-) create mode 100644 testdata/server/get_server_information_response_failed_no_auth.xml create mode 100644 testdata/usage/get_space_response_failed_no_auth.xml create mode 100644 testdata/usage/get_space_usage_response_failed_no_auth.xml create mode 100644 testdata/usage/get_traffic_response_failed_no_auth.xml diff --git a/internal/server/server_test.go b/internal/server/server_test.go index f5feb6d..f1cca41 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -11,7 +11,7 @@ import ( func TestDecodeServices(t *testing.T) { t.Parallel() - resp := testutil.DecodeFixture(t, "account/get_server_information_response_success.xml") + resp := testutil.DecodeFixture(t, "server/get_server_information_response_success.xml") got, err := server.DecodeServices(resp.Body.ReturnInfo) if err != nil { t.Fatalf("DecodeServices: %v", err) @@ -33,7 +33,7 @@ func TestDecodeServices(t *testing.T) { func TestClientInformation(t *testing.T) { t.Parallel() - resp := testutil.DecodeFixture(t, "account/get_server_information_response_success.xml") + resp := testutil.DecodeFixture(t, "server/get_server_information_response_success.xml") c := server.NewClient(&testutil.FakeCaller{Resp: resp}) list, err := c.Information(context.Background()) if err != nil { @@ -53,9 +53,19 @@ func TestClientInformationPropagatesError(t *testing.T) { } } +// TestFaultFixturesDecodeToDocumentedCodes binds the captured +// *_response_failed_*.xml fixtures to the KAS contract via the shared +// testutil.AssertFaultFixtures anchor. +func TestFaultFixturesDecodeToDocumentedCodes(t *testing.T) { + t.Parallel() + testutil.AssertFaultFixtures(t, "server", map[string]string{ + "get_server_information_response_failed_no_auth.xml": "no_auth", + }) +} + func TestServiceListTabular(t *testing.T) { t.Parallel() - resp := testutil.DecodeFixture(t, "account/get_server_information_response_success.xml") + resp := testutil.DecodeFixture(t, "server/get_server_information_response_success.xml") list, _ := server.DecodeServices(resp.Body.ReturnInfo) rows := list.TableRows() if len(rows) != 8 { diff --git a/internal/soap/soap_test.go b/internal/soap/soap_test.go index 12c4eb0..9b25961 100644 --- a/internal/soap/soap_test.go +++ b/internal/soap/soap_test.go @@ -121,7 +121,7 @@ func TestDecodeGetAccountsShape(t *testing.T) { // TestDecodeGetServerInformationShape exercises the array-of-maps shape // where ReturnInfo lists installed services. func TestDecodeGetServerInformationShape(t *testing.T) { - resp, err := decodeFile(t, filepath.Join(testutil.RepoRoot(t), "testdata/account/get_server_information_response_success.xml")) + resp, err := decodeFile(t, filepath.Join(testutil.RepoRoot(t), "testdata/server/get_server_information_response_success.xml")) if err != nil { t.Fatalf("Decode: %v", err) } diff --git a/internal/usage/usage_test.go b/internal/usage/usage_test.go index 715a536..35f2e32 100644 --- a/internal/usage/usage_test.go +++ b/internal/usage/usage_test.go @@ -133,6 +133,18 @@ func TestDecodeTrafficRejectsMalformedYear(t *testing.T) { } } +// TestFaultFixturesDecodeToDocumentedCodes binds the captured +// *_response_failed_*.xml fixtures to the KAS contract via the shared +// testutil.AssertFaultFixtures anchor. +func TestFaultFixturesDecodeToDocumentedCodes(t *testing.T) { + t.Parallel() + testutil.AssertFaultFixtures(t, "usage", map[string]string{ + "get_space_response_failed_no_auth.xml": "no_auth", + "get_space_usage_response_failed_no_auth.xml": "no_auth", + "get_traffic_response_failed_no_auth.xml": "no_auth", + }) +} + func TestClientSpace(t *testing.T) { t.Parallel() resp := testutil.DecodeFixture(t, "usage/get_space_response_success.xml") diff --git a/testdata/server/get_server_information_response_failed_no_auth.xml b/testdata/server/get_server_information_response_failed_no_auth.xml new file mode 100644 index 0000000..9e1bcb3 --- /dev/null +++ b/testdata/server/get_server_information_response_failed_no_auth.xml @@ -0,0 +1,11 @@ + + + + + SOAP-ENV:Server + no_auth + KasApi + got no kas_login for authentication + + + \ No newline at end of file diff --git a/testdata/usage/get_space_response_failed_no_auth.xml b/testdata/usage/get_space_response_failed_no_auth.xml new file mode 100644 index 0000000..9e1bcb3 --- /dev/null +++ b/testdata/usage/get_space_response_failed_no_auth.xml @@ -0,0 +1,11 @@ + + + + + SOAP-ENV:Server + no_auth + KasApi + got no kas_login for authentication + + + \ No newline at end of file diff --git a/testdata/usage/get_space_usage_response_failed_no_auth.xml b/testdata/usage/get_space_usage_response_failed_no_auth.xml new file mode 100644 index 0000000..9e1bcb3 --- /dev/null +++ b/testdata/usage/get_space_usage_response_failed_no_auth.xml @@ -0,0 +1,11 @@ + + + + + SOAP-ENV:Server + no_auth + KasApi + got no kas_login for authentication + + + \ No newline at end of file diff --git a/testdata/usage/get_traffic_response_failed_no_auth.xml b/testdata/usage/get_traffic_response_failed_no_auth.xml new file mode 100644 index 0000000..9e1bcb3 --- /dev/null +++ b/testdata/usage/get_traffic_response_failed_no_auth.xml @@ -0,0 +1,11 @@ + + + + + SOAP-ENV:Server + no_auth + KasApi + got no kas_login for authentication + + + \ No newline at end of file From 79a08fa17296621431fe7c1b674ee4f5ca46c7fd Mon Sep 17 00:00:00 2001 From: Alexander Saal Date: Sat, 18 Jul 2026 07:47:38 +0200 Subject: [PATCH 7/7] docs: fix ROADMAP server entry, consolidate CHANGELOG unreleased block ROADMAP.md listed 'server get'; the shipped command is 'server info'. CHANGELOG.md's [Unreleased] had duplicated, unordered subsections (Changed x3, Fixed x2); they are consolidated into one block per type in canonical Keep-a-Changelog order, plus entries for the Med-severity review fixes. --- CHANGELOG.md | 271 +++++++++++++++++++++++++++++---------------------- ROADMAP.md | 2 +- 2 files changed, 153 insertions(+), 120 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f531641..00c37cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,112 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- Directory protection write endpoints (#123, #13 write slice): + `kasapi-cli directoryprotection add --password + [--authname ]`, `… update [--password ] + [--authname ]` and `… delete ` wire + `add_directoryprotection` / `update_directoryprotection` / + `delete_directoryprotection`. A protection entry is identified by the + `(path, user)` pair taken as two positional arguments (a single path + can protect several users). `update` and `delete` are gated by the + #109 confirmation prompt — `update` replaces the access password (the + previous one is unrecoverable) and `delete` revokes access, so both + can lock users out; `add` is reversible and not prompted. All three + honour `--dry-run` (#132) and emit a #131 audit record; + `directory_password` is redacted in both sinks. There is **no** + `_new_password` split — `update` sends the replacement under the same + `directory_password` key `add` uses — and `update` sends only the + explicitly-changed `--password`/`--authname` (keyed on cobra + `Changed`), so an omitted password keeps the current one. KAS also + accepts parallel `directory_user`/`directory_password` arrays to + create several protected users in one call (hence the + `directory_user_count_neq_passcount` fault); the captured request + fixtures only exercise the scalar single-user form, and the array + wire-encoding is not captured, so this slice deliberately models one + `(path, user)` protection per call rather than inventing the array + shape. + +- Mail standard filter write endpoints (#116, #13 write slice): + `kasapi-cli mail filters add --filter [--filter + ...]` and `… delete ` wire + `add_mailstandardfilter` / `delete_mailstandardfilter`. Both are + gated by the #109 confirmation prompt: the KAS API has no + `update_mailstandardfilter` action, so `add` *replaces* the configured + filter chain wholesale (items previously set but missing from the new + `--filter` list are dropped), which is destructive to recover from + without a stored copy. Both honour `--dry-run` (#132) and emit a #131 + audit record. Repeatable `--filter` items are joined with `;` on the + wire (the format the captured `add_mailstandardfilter` request + fixture uses); each item is either a bare filter id (e.g. `pdw`) or + `: