diff --git a/doc/cli.md b/doc/cli.md index 32513a2..77ee278 100644 --- a/doc/cli.md +++ b/doc/cli.md @@ -26,7 +26,7 @@ pipelines have [their own flags](#pipelines): ``` --= e.g. --logging.level=debug -- e.g. --logging.level debug --- bare flag, means true +-- bare flag: true, for a boolean path only ``` ### Common diff --git a/doc/configuration.md b/doc/configuration.md index 4018d6f..74e1198 100644 --- a/doc/configuration.md +++ b/doc/configuration.md @@ -231,13 +231,18 @@ lw --logging.level debug # space form also works lw --quiet # bare flag means true ``` -Unrecognized flags are reported on stderr before the logger exists and are then -ignored: +A bare flag is accepted only for a boolean key: `--logging.file.directory +--quiet` fails with `--logging.file.directory needs a value`. Unrecognized flags, +positional words and single-dash arguments are reported on stderr before the +logger exists and are then ignored: ``` -Warning: unrecognized flags ignored: [pipelines.0.name] +Warning: unrecognized flags ignored: [pipelines.0.name -q] ``` +An empty `LOGWISP_` variable counts as unset, as the pipeline variables +do. + Array-indexed paths such as `--pipelines.0.name=x` are unrecognized; whole pipelines have [their own flags](cli.md#pipelines). diff --git a/doc/security.md b/doc/security.md index 338603f..93a0f62 100644 --- a/doc/security.md +++ b/doc/security.md @@ -26,6 +26,16 @@ configure it, and — equally important — what it does not yet do. [Startup Warnings](#startup-warnings). - Unknown configuration keys rejected: a typo in `tls`, `auth` or a table path fails startup. +- Configuration input hardened (lixenwraith/config v0.2.2, reviewed with + adversarial tests and fuzzing like `auth`): + - a string in the file is one list entry, so a filter pattern holding a + comma is not split into patterns that match nothing; + - the file is opened without blocking, so a FIFO swapped in for it cannot + hang the watcher or a `SIGHUP` reload; + - a permission change on the file is reported once and does not stop + auto-reload; + - conversion errors name the key, never the value, so a secret placed in + the wrong key does not reach the log. **Not implemented:** diff --git a/doc/todo.md b/doc/todo.md index c960705..59dfb9f 100644 --- a/doc/todo.md +++ b/doc/todo.md @@ -131,34 +131,12 @@ its input queue into the client queues, then let every client writer drain its queue within a bound (`write_timeout_ms`) before the disconnect frame. Verify with a finite stdin into each sink and a connected client counting lines. -## 3. Hardening review of `lixenwraith/config` and `lixenwraith/toml` +## 3. Bump `lixenwraith/toml` -Both parse untrusted-shaped input (config files, environment, command line) -and were not reviewed with the auth work. Review them as `auth` was reviewed: -adversarial lenses, reproduction before belief, a fix per confirmed finding in -the library repository, then a dependency bump here. - -- toml parser: - - Limits on nesting depth, key count, string and array length, and number - sizes. - - Behaviour on invalid UTF-8, duplicate keys and tables, dotted-key and - inline-table redefinition. - - Fuzz targets (`go test -fuzz`) seeded with the TOML test suite. -- config: - - The weakly typed decoder: string-to-number overflow, duration and size - parsing, the comma split into slices. - - `maxValueDepth` and `MaxValueSize`, environment and CLI precedence, and - unknown-key reporting. -- Files: - - `PreventPathTraversal`, symlink handling and the file-size cap. - - The watcher: polling, debounce, a file replaced by rename or symlink swap, - TOCTOU between stat and read. -- Errors: values must never echo secrets (a misplaced password in a config - value reported back in an error). -- Deliverables: - - Findings per library. - - Fuzz targets committed in the library repositories. - - A note in this repository's security.md once done. +Its hardening commits (a table budget, TOML 1.1 escapes, control characters +refused outside strings, bounded errors that never print a value) wait to merge +in that repository. Then bump it here and drop this item; security.md's library +note already covers it. ## 4. Packaging: AUR, FreeBSD ports, Debian diff --git a/go.mod b/go.mod index 0ee85ac..5ba3877 100644 --- a/go.mod +++ b/go.mod @@ -4,9 +4,9 @@ go 1.27.1 require ( github.com/lixenwraith/auth v0.0.0-20261002191639-3ed0e995807c - github.com/lixenwraith/config v0.2.1 + github.com/lixenwraith/config v0.2.2 github.com/lixenwraith/log v0.1.11 - github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0 + github.com/lixenwraith/toml v0.1.3 golang.org/x/term v0.46.0 ) diff --git a/go.sum b/go.sum index 55c23a0..c3d8f73 100644 --- a/go.sum +++ b/go.sum @@ -2,12 +2,12 @@ github.com/golang-jwt/jwt/v5 v5.3.1 h1:kYf81DTWFe7t+1VvL7eS+jKFVWaUnK9cB1qbwn63Y github.com/golang-jwt/jwt/v5 v5.3.1/go.mod h1:fxCRLWMO43lRc8nhHWY6LGqRcf+1gQWArsqaEUEa5bE= github.com/lixenwraith/auth v0.0.0-20261002191639-3ed0e995807c h1:zwpVa85g267vo13UCzCao4lfVgUPaU79CmxPW9OdF8w= github.com/lixenwraith/auth v0.0.0-20261002191639-3ed0e995807c/go.mod h1:411JUqnbQgy9jgoFV+fH91TfVUEfzYA4dyPv7f4c264= -github.com/lixenwraith/config v0.2.1 h1:a1VLWhtAi1F7FWtRtFVpPkvemnBEf+4xrvDupvwHYtg= -github.com/lixenwraith/config v0.2.1/go.mod h1:yfJQwKS+/6Dg70GIoHeYc0eYztOPETpqe6nUIbakcyg= +github.com/lixenwraith/config v0.2.2 h1:X+l3Tf8Rbk49cipYBJU7/pfdw5wDwD4hRxtFH7rZgOc= +github.com/lixenwraith/config v0.2.2/go.mod h1:yfJQwKS+/6Dg70GIoHeYc0eYztOPETpqe6nUIbakcyg= github.com/lixenwraith/log v0.1.11 h1:dKWLnaR8dhXxj4l/aU5XZiFIsr8geaJQbOf8m7SiKhU= github.com/lixenwraith/log v0.1.11/go.mod h1:EE8i5nSAW2yZBTytKuhdgehmgzrby6kW1k5DyH1SvBg= -github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0 h1:ue1pILlQAovrxTMSaKII4AbZ7yY4mNs3cyivoSBwMmA= -github.com/lixenwraith/toml v0.0.0-20261001212158-cede215365c0/go.mod h1:itcT/Vt5mwmX9KVGCMsWZr6vbU/JgZ24wV/I0J5cMt0= +github.com/lixenwraith/toml v0.1.3 h1:mhnNVLNogdaqjGd3eC4rV9ZqY3j1Sv+DK1erZ+SPFeI= +github.com/lixenwraith/toml v0.1.3/go.mod h1:itcT/Vt5mwmX9KVGCMsWZr6vbU/JgZ24wV/I0J5cMt0= golang.org/x/crypto v0.57.0 h1:3ZVCjf8Ggz7zneR/EHRVx68Ctf+2pmIMP2UFhh9cC6M= golang.org/x/crypto v0.57.0/go.mod h1:Fdz0i5U6CoizGwLda9DttjSk6qlZo25zYNtR+ycvuZA= golang.org/x/sys v0.48.0 h1:bbX/i/6MgT9BVLM9RT1thmxL04yeTAhbEz4SyadbXoo= diff --git a/internal/config/loader_test.go b/internal/config/loader_test.go index d7e84b7..4c60653 100644 --- a/internal/config/loader_test.go +++ b/internal/config/loader_test.go @@ -301,3 +301,21 @@ func TestNoConfigFileMeansQuietLogging(t *testing.T) { t.Fatalf("environment over the file-less default: %+v %v", cfg, err) } } + +// A string in the file is one list entry: split at its comma, an exclude +// pattern such as password=\S{8,64} would become two that match nothing +func TestFileStringIsOneListEntry(t *testing.T) { + isolateConfig(t) + testutil.WriteFile(t, "logwisp.toml", "[[pipelines]]\nname = \"p\"\n"+ + "[[pipelines.plugin_sources]]\nid = \"in\"\ntype = \"null\"\n[[pipelines.plugin_sinks]]\nid = \"out\"\ntype = \"null\"\n"+ + "[[pipelines.flow.filters]]\ntype = \"exclude\"\npatterns = \"password=\\\\S{8,64}\"\n") + m, err := Load(nil) + if err != nil { + t.Fatal(err) + } + defer m.Close() + cfg, err := m.Snapshot() + if err != nil || !reflect.DeepEqual(cfg.Pipelines[0].Flow.Filters[0].Patterns, []string{`password=\S{8,64}`}) { + t.Fatalf("patterns: %v %v", cfg.Pipelines[0].Flow.Filters, err) + } +} diff --git a/internal/config/validate.go b/internal/config/validate.go index ab89cd3..3263266 100644 --- a/internal/config/validate.go +++ b/internal/config/validate.go @@ -111,9 +111,7 @@ func Scan(configMap map[string]any, target any) error { } // checkKeys walks nested tables against the toml tags of t, refusing unknown -// keys and making a lone string for a list field a one-entry list: the weakly -// typed decoder would split it at commas, which a pattern or a DN may hold. -// Map-typed fields hold free-form keys and are not descended into. +// keys. Map-typed fields hold free-form keys and are not descended into. func checkKeys(m map[string]any, t reflect.Type, prefix string) error { for t.Kind() == reflect.Pointer || t.Kind() == reflect.Slice { t = t.Elem() @@ -134,10 +132,6 @@ func checkKeys(m map[string]any, t reflect.Type, prefix string) error { } var tables []map[string]any switch v := m[key].(type) { - case string: - if ft.Kind() == reflect.Slice && ft.Elem().Kind() == reflect.String { - m[key] = []string{v} - } case map[string]any: if err := checkKeys(v, ft, prefix+key+"."); err != nil { return err