fix: address Med-severity findings from the whole-codebase review

ROADMAP.md

@@ -32,7 +32,7 @@ Cross-cutting prerequisites for the v0.2.0 write phase — these are not KAS-API
 - [x] `accounts list` / `accounts get <account-login>` (`get_accounts`, with `account_login` filter)
 - [x] `accounts settings` / `accounts resources` (`get_accountsettings`, `get_accountresources`)
 - [ ] Account write paths (`add_account`, `update_account`, `delete_account`, `update_accountsettings`, `update_superusersettings`, #110)
-- [x] `server get` (`get_server_information`)
+- [x] `server info` (`get_server_information`)
 
 ## Usage
 

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

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)
 		}
 	}
 }

internal/cli/audit.go

@@ -66,6 +66,12 @@ var auditSecretParams = map[string]struct{}{
 
 const auditRedacted = "<redacted>"
 
+// 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 "<elided N bytes>" 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("<elided %d bytes>", 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 `""`

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"] != "<elided 25 bytes>" {
+		t.Errorf("config = %q, want <elided 25 bytes>", got["config"])
+	}
+	if got["subscriber"] != "<elided 13 bytes>" {
+		t.Errorf("subscriber = %q, want <elided 13 bytes>", got["subscriber"])
+	}
+	if got["long"] != "<elided 300 bytes>" {
+		t.Errorf("long = %q, want <elided 300 bytes>", 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)

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

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": "<elided 13 bytes>"},
 			[]string{"is_active"},
 		},
 	}

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}} }

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())
+	}
+}

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",
 	})
 }

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 {