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

CHANGELOG.md

@@ -115,7 +115,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
-- `testdata/cronjob/{add_cronjob_response_success,add_cronjob_response_warning,update_cronjob_response_success}.xml`
+- `testdata/cronjob/{add_cronjob_response_success,add_cronjob_response_success_warning,update_cronjob_response_success}.xml`
   carry a top-of-file XML comment documenting that KAS itself echoes
   the notification address under `mail_address` (double d) in the
   `KasRequestParams` echo block, while the documented request key is
@@ -575,6 +575,138 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+- Whole-codebase re-review follow-up (third pass, High + Med + Low
+  findings):
+  - Group commands (`mail`, `accounts`, `config`, ...) invoked with an
+    unknown subcommand no longer print help and exit 0 — a typo'd
+    subcommand read as success to scripts. `cli.Finalize` now gives
+    every non-runnable group an explicit unknown-subcommand rejection
+    (exit 1, with cobra's "Did you mean this?" suggestions); a bare
+    group invocation keeps printing help with exit 0.
+  - The lazily-registered `completion` command is registered before the
+    exit-code walkers run, so its args-validation failures exit 1 (user
+    error) instead of 2.
+  - The success audit record of create actions whose identifier KAS
+    generates server-side (`add_ftpuser`, `add_database`,
+    `add_sambauser`, `add_ddnsuser`, `add_cronjob`, `add_mailaccount`,
+    `add_mailforward`, `add_mailinglist`) now carries the assigned
+    identifier as `created_id`, so the create correlates with the
+    identifier later update/delete records carry as their target.
+  - A confirmation-prompt I/O failure (neither a yes nor a no was read)
+    now leaves an audit record with the new outcome `aborted` instead
+    of silently skipping the trace of a blocked destructive attempt.
+  - The transport 5xx fault sniff also recognises a prefix-less
+    default-namespace `<Fault>` element, matching what the SOAP decoder
+    accepts; an end-to-end api-layer test pins that a fault delivered
+    with HTTP 500 surfaces as the same typed `*api.Error` a 200-wrapped
+    fault produces.
+  - `sambausers update` binds its own replacement-flag set instead of
+    sharing `add`'s (whose `--help` texts claimed "required for add") —
+    the same add/update split the cronjob/ftpuser slices got in the
+    second pass.
+  - The remaining `get_<singular>` fixtures whose filename encoded a
+    non-existent singular KAS action while embedding the plural one
+    were renamed to `<real_action>_<kind>_<variant>.xml` across
+    `testdata/{cronjob,ddns,domain,ftpuser,mailaccount,mailforward,mailinglist,sambauser}/`
+    (e.g. `get_ftpuser_response_success.xml` →
+    `get_ftpusers_response_success_single.xml`), and the convention
+    example in `CLAUDE.md` now uses a conforming name.
+
+- Whole-codebase re-review follow-up (second pass, Med + Low findings):
+  - Bad user input now consistently exits 1: cobra positional-args
+    failures (e.g. a missing required argument) and an unknown root
+    subcommand were falling through to the API-error exit 2. The
+    command tree wraps every `Args` validator via
+    `cli.MarkArgErrorsAsUserErrors` and the root command classifies
+    unknown subcommands itself.
+  - `transport.Client.Do` no longer blindly retries a 5xx response
+    whose body carries a SOAP fault: the body is passed through to the
+    decoder so the typed-fault path (auth refresh, flood fallback,
+    exit-code classification) applies. Fault-free 5xx bodies stay
+    retryable.
+  - `testutil.AssertFaultFixtures` fails on `want` entries that match
+    no fixture file on disk, so a renamed or deleted fixture cannot
+    leave a dead pin behind; the shared top-level
+    `response_failed_*.xml` set is now anchored by a dedicated test in
+    `internal/api` pinning each fixture to its `api.Code*` constant.
+  - `config.Resolve` with a nil config and `--profile` wraps
+    `ErrUnknownProfile` instead of returning a string-only error.
+  - `session.Store.Load` treats an entry without `expires_at` (only
+    producible by hand-editing sessions.toml) as expired instead of
+    never-expiring.
+  - `cronjobs update` / `ftpusers update` bind their own flag sets
+    instead of sharing the add flags, so `--help` no longer advertises
+    add defaults (`https`, `*`, `default`) or "required for add" texts
+    — matching the database/mailaccount/ddnsuser/mailinglist split.
+  - The `--dry-run` help text says "write command" instead of
+    "destructive command" — the flag covers non-gated `add` writes too.
+  - `testdata/domain/get_topleveldomains_request.xml` carried
+    `kas_action: get_subdomains` (mis-captured copy); corrected to the
+    action the filename and the response fixture encode.
+  - Fixture names aligned with the documented convention:
+    `dns/get_dns_settings_{request,response_success}_zone_host_and_record_id.xml`
+    (variant after kind) and
+    `cronjob/add_cronjob_response_success_warning.xml` (a success
+    variant, not a distinct status).
+  - `internal/{ftpuser,cronjob,mailaccount}/doc.go` no longer name
+    non-existent singular get actions; the stale ftpuser "verify
+    against the live API" note now records the #119 verification
+    result. The `internal/ddns` `in_progress` comment no longer claims
+    fixture backing the captured fixtures do not contain.
+  - Docs refreshed to the shipped state: README status/CI/what-it-does
+    (write slices are live, not "pending"), the destructive-writes
+    prompt example (single line, `permanently delete` verb, stderr
+    note), the audit-trace scope (the `sessions delete` / `config
+    use-profile` session logout is explicitly outside the pipeline),
+    `ROADMAP.md` (`mail lists get`, `<ddns-login>` placeholder), and
+    the CLAUDE.md repository-state paragraph.
+
+- Whole-codebase review follow-up (Low findings):
+  - `api.TokenSource.Invalidate` now reports whether the next
+    Credentials call can produce fresh credentials; `api.Client` skips
+    the auth-failure retry for a `StaticTokenSource` instead of
+    doubling the failing request with identical credentials.
+  - `transport.Client` no longer classifies `context.Canceled` /
+    `DeadlineExceeded` as retryable — the caller's cancellation is not
+    a transient server condition.
+  - The destructive-write `[y/N]` prompt goes to **stderr** so a
+    redirected stdout cannot swallow the question.
+  - A dispatched write whose audit sink fails keeps its true exit
+    classification: a KAS fault stays exit 2 (audit failure reported as
+    a warning), and a successful write renders its result before the
+    audit error surfaces as exit 1.
+  - Refused (`outcome=refused`, non-TTY without `--yes`) and declined
+    (`outcome=declined`, prompt answered no) destructive attempts now
+    leave an audit record; previously only dispatched writes and
+    dry-runs were traced. Documented in
+    `docs/usage/destructive-writes.md`.
+  - `auth.DecodeResponse` validates the credential token shape (40
+    alphanumeric characters) before it is cached and persisted; the
+    error reports only the length, never the content.
+  - `auth.EncodeRequest` range-checks `Lifetime` against the documented
+    1..30000 session_lifetime bound (0 = server default).
+  - New `config.ErrUnknownProfile` / `config.ErrMissingCredentials`
+    sentinels; `Resolve` failures are `errors.Is`-able instead of
+    string-matchable only.
+  - `dns` / `directoryprotection` / `server` / `usage` now keep their
+    `Caller` unexported (`c`), matching the other ten modules; the
+    exported `API` field was construction surface no caller used.
+  - `cronjob.FieldID` replaces three hardcoded `"cronjob_id"` literals.
+  - `internal/{softwareinstall,directoryprotection,database}/doc.go`
+    no longer name non-existent KAS actions (`get_softwareinstalls`,
+    `get_directoryprotections`, `get_database`); fixtures encoding
+    those non-actions in their filenames were renamed to the real
+    action plus a variant suffix
+    (`get_databases_{request,response_success}_single.xml`,
+    `get_directoryprotection_{request,response_success}_all.xml`,
+    `get_softwareinstall_{request,response_success}_all.xml`).
+  - The captured `testdata/{chown,ssl,symlink}/` fault fixtures (#125
+    placeholders) are now anchored by one-line
+    `testutil.AssertFaultFixtures` tests instead of sitting
+    unreferenced.
+  - Fixture values `info@example1.org` and `/example-new.com/` replaced
+    with RFC 2606-reserved names (`example.org`, `example.net`).
+
 - Whole-codebase review follow-up (Med findings):
   - `transport.Client` now enforces the 16-MB `soap.MaxResponseBytes`
     cap at the HTTP body read. Previously the cap lived only in the

CLAUDE.md

@@ -4,9 +4,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co
 
 ## Repository State
 
-The read-phase modules are wired up (accounts, server, domains/subdomains/TLDs, DNS, mail, databases, FTP/Samba users, cronjobs, directory protection, software installs, DDNS users, usage statistics) and the standard CI gate (`lint & test` + `docs sync`) runs on every push and pull request. `main` is protected: signed commits are required, force-pushes to `main` are blocked for the GitHub UI / `gh pr merge --rebase` (signatures are stripped server-side), and merging happens via a locally-rebased fast-forward push by the maintainer (see `.claude/skills/kasapi-cli-git-workflow/SKILL.md`).
+The read-phase modules are wired up (accounts, server, domains/subdomains/TLDs, DNS, mail, databases, FTP/Samba users, cronjobs, directory protection, software installs, DDNS users, usage statistics), and so are most v0.2.0 write slices (mail accounts/forwards/filters/lists, databases, FTP/Samba users, DDNS users, cronjobs, directory protection) including the destructive-write safety contract (confirmation gate, `--dry-run`, audit records — see `docs/usage/destructive-writes.md`). The CI gate (`lint & test`, `docs sync`, `goreleaser` config check, `govulncheck`, CodeQL) runs on every push and pull request. `main` is protected: signed commits are required, force-pushes to `main` are blocked for the GitHub UI / `gh pr merge --rebase` (signatures are stripped server-side), and merging happens via a locally-rebased fast-forward push by the maintainer (see `.claude/skills/kasapi-cli-git-workflow/SKILL.md`).
 
-Write paths and the remaining read endpoints are part of the v0.2.0 backlog tracked on the *kasapi-cli v0.1.0* GitHub project; do not invent endpoints not documented at <https://kasapi.kasserver.com/dokumentation/phpdoc/>.
+The remaining write endpoints (software installs, filesystem/SSL helpers) are part of the v0.2.0 backlog tracked on the *kasapi-cli v0.1.0* GitHub project; do not invent endpoints not documented at <https://kasapi.kasserver.com/dokumentation/phpdoc/>.
 
 There is no predecessor library and no inherited backlog. Do not assume or import patterns from any other KAS client; design from the KAS API docs and the fixtures in `testdata/`.
 
@@ -31,7 +31,7 @@ Filename convention:
 - One file per `(kas_action, kind)` pair: `<kas_action>_<kind>[_<variant>].xml`, where `kind` is `request` or `response_<status>`, and `status` is `success` or `failed`. Examples:
     - `get_accounts_response_success.xml`
     - `add_account_response_failed_account_kas_password_syntax_incorrect.xml`
-    - `get_ftpuser_response_success_empty_list.xml` (variant of the success shape)
+    - `get_ftpusers_response_success_empty_list.xml` (variant of the success shape; the `<kas_action>` prefix is always the real — plural — action the file embeds, never a singular alias)
 
 Add a new fixture whenever a new KAS call is wired up; redact secrets before committing.
 

README.md

@@ -22,13 +22,13 @@
 
 ## Status
 
-Early development. The transport, authentication, configuration, and the v0.1.0 read modules are wired up; write paths and the remaining read endpoints are still pending — see [ROADMAP.md](ROADMAP.md) for the current state. The repository also ships recorded KAS-API response fixtures under `testdata/` that drive offline parser tests.
+Early development. The transport, authentication, configuration, the v0.1.0 read modules, and most of the v0.2.0 write paths (mail accounts/forwards/filters/lists, databases, FTP/Samba users, DDNS users, cronjobs, directory protection) are wired up. The remaining write endpoints (software installs, the filesystem/SSL helpers) are still pending — see [ROADMAP.md](ROADMAP.md) for the current state. The repository also ships recorded KAS-API response fixtures under `testdata/` that drive offline parser tests.
 
-CI gates `gofmt`/`go vet`/`golangci-lint` (with `gosec`)/`go test`/`go test -race`/`go build`, plus a `govulncheck` job on every PR. Dependabot keeps `gomod` and `github-actions` versions current.
+CI gates `gofmt`/`go vet`/`golangci-lint` (with `gosec`)/`go test`/`go test -race`/`go build`, a docs-sync job (`make docs` must produce no diff against the checked-in `docs/cli/`), a `goreleaser` config check, CodeQL, plus a `govulncheck` job on every PR. Dependabot keeps `gomod` and `github-actions` versions current.
 
 ## What it does
 
-`kasapi-cli` is a command-line client for the All-Inkl KAS-API. It wraps the SOAP/`ns2:Map` wire format the API uses, handles the `KasAuth` credential-token flow (plain or session, optional 2FA), enforces the `KasFloodDelay` between calls, and exposes read operations for the resources documented at <https://kasapi.kasserver.com/dokumentation/phpdoc/>. Write paths are scheduled for v0.2.0 — see [ROADMAP.md](ROADMAP.md).
+`kasapi-cli` is a command-line client for the All-Inkl KAS-API. It wraps the SOAP/`ns2:Map` wire format the API uses, handles the `KasAuth` credential-token flow (plain or session, optional 2FA), enforces the `KasFloodDelay` between calls, and exposes read and write operations for the resources documented at <https://kasapi.kasserver.com/dokumentation/phpdoc/>. Destructive writes are gated behind a confirmation prompt, support `--dry-run`, and leave an audit trace — see [docs/usage/destructive-writes.md](docs/usage/destructive-writes.md). The remaining write endpoints are tracked in [ROADMAP.md](ROADMAP.md).
 
 ## Install
 

ROADMAP.md

@@ -58,7 +58,7 @@ Cross-cutting prerequisites for the v0.2.0 write phase — these are not KAS-API
 - [x] `mail forwards add/update/delete` (`add_mailforward`, `update_mailforward`, `delete_mailforward`, #115)
 - [x] `mail filters list` (`get_mailstandardfilter`)
 - [x] `mail filters add/delete` (`add_mailstandardfilter`, `delete_mailstandardfilter`, #116)
-- [x] `mail lists list` (`get_mailinglists`)
+- [x] `mail lists list` / `mail lists get <mailinglist-name>` (`get_mailinglists`)
 - [x] `mail lists add/update/delete` (`add_mailinglist`, `update_mailinglist`, `delete_mailinglist`, #117)
 
 ## Hosting resources
@@ -69,7 +69,7 @@ Cross-cutting prerequisites for the v0.2.0 write phase — these are not KAS-API
 - [x] `ftpusers add/update/delete` (`add_ftpuser`, `update_ftpuser`, `delete_ftpuser`, #119)
 - [x] `sambausers list` / `sambausers get <samba-login>` (`get_sambausers`, with `samba_login` filter)
 - [x] `sambausers add/update/delete` (`add_sambauser`, `update_sambauser`, `delete_sambauser`, #120)
-- [x] `ddnsusers list` / `ddnsusers get <dyndns-login>` (`get_ddnsusers`, with `ddns_login` filter)
+- [x] `ddnsusers list` / `ddnsusers get <ddns-login>` (`get_ddnsusers`, with `ddns_login` filter)
 - [x] `ddnsusers add/update/delete` (`add_ddnsuser`, `update_ddnsuser`, `delete_ddnsuser`, #121)
 - [x] `cronjobs list` / `cronjobs get <cronjob-id>` (`get_cronjobs`, with `cronjob_id` filter)
 - [x] `cronjobs add/update/delete` (`add_cronjob`, `update_cronjob`, `delete_cronjob`, #118)

cmd/kasapi-cli/main.go

@@ -34,6 +34,7 @@ func main() {
 		cli.NewConfigCmd(opts),
 		cli.NewGenDocsCmd(),
 	)
+	cli.Finalize(root)
 	if err := root.Execute(); err != nil {
 		fmt.Fprintln(os.Stderr, "kasapi-cli:", err)
 		os.Exit(cli.CodeFor(err))

docs/cli/kasapi-cli.md

@@ -17,7 +17,7 @@ kasapi-cli [flags]
       --auth-data string                 KAS auth data (overrides config and KAS_AUTHDATA)
       --auth-type string                 KAS auth strategy: 'plain' = send password on each KasApi call (no KasAuth, no 2FA support); 'session' = bootstrap via KasAuth and reuse the credential token. Overrides config and KAS_AUTHTYPE.
       --config string                    path to the kasapi-cli config file (overrides the default location)
-      --dry-run                          preview a destructive command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
+      --dry-run                          preview a write command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
   -h, --help                             help for kasapi-cli
       --login string                     KAS login (overrides config and KAS_LOGIN)
       --otp string                       2FA one-time PIN — sent to KasAuth as session_2fa during the credential-token bootstrap. Requires auth_type=session; the KAS API does not document 2FA on direct kas_auth_type=plain calls.

docs/cli/kasapi-cli_accounts.md

@@ -2,6 +2,10 @@
 
 Inspect KAS accounts owned by the authenticated login
 
+```
+kasapi-cli accounts [flags]
+```
+
 ### Options
 
 ```
@@ -15,7 +19,7 @@ Inspect KAS accounts owned by the authenticated login
       --auth-data string                 KAS auth data (overrides config and KAS_AUTHDATA)
       --auth-type string                 KAS auth strategy: 'plain' = send password on each KasApi call (no KasAuth, no 2FA support); 'session' = bootstrap via KasAuth and reuse the credential token. Overrides config and KAS_AUTHTYPE.
       --config string                    path to the kasapi-cli config file (overrides the default location)
-      --dry-run                          preview a destructive command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
+      --dry-run                          preview a write command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
       --login string                     KAS login (overrides config and KAS_LOGIN)
       --otp string                       2FA one-time PIN — sent to KasAuth as session_2fa during the credential-token bootstrap. Requires auth_type=session; the KAS API does not document 2FA on direct kas_auth_type=plain calls.
   -o, --output string                    output format: json|yaml|table (default table)

docs/cli/kasapi-cli_accounts_get.md

@@ -19,7 +19,7 @@ kasapi-cli accounts get <account-login> [flags]
       --auth-data string                 KAS auth data (overrides config and KAS_AUTHDATA)
       --auth-type string                 KAS auth strategy: 'plain' = send password on each KasApi call (no KasAuth, no 2FA support); 'session' = bootstrap via KasAuth and reuse the credential token. Overrides config and KAS_AUTHTYPE.
       --config string                    path to the kasapi-cli config file (overrides the default location)
-      --dry-run                          preview a destructive command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
+      --dry-run                          preview a write command's KAS request (action + redacted parameters) and exit 0 without dispatching or prompting; honours --output
       --login string                     KAS login (overrides config and KAS_LOGIN)
       --otp string                       2FA one-time PIN — sent to KasAuth as session_2fa during the credential-token bootstrap. Requires auth_type=session; the KAS API does not document 2FA on direct kas_auth_type=plain calls.
   -o, --output string                    output format: json|yaml|table (default table)