bug: non-file outputs accept output.path and ignore it silently #1066

Activity

1. strawgate

   added

   bugSomething isn't workingSomething isn't working

   on Apr 5, 2026
2. github-actions commented on Apr 5, 2026

   

   github-actionsBot

   on Apr 5, 2026 – with GitHub Actions

   Contributor

   More actions

   tl;dr I can reproduce this on master: output.path is currently accepted for multiple non-file outputs and ignored at runtime; the clean fix is to fail validation when path is set for output types that do not consume it.

   Recommendation

   Add an explicit validation rule in Config::validate() that rejects output.path for non-file outputs that do not use it (stdout, null, otlp, http, loki, tcp, udp, arrow_ipc).

   For elasticsearch, there is one nuance: runtime currently treats path as an alias for index. I recommend handling this explicitly (either keep as documented alias, or deprecate/remove alias and require index).

   Findings

   • OutputConfig exposes path for all outputs: crates/logfwd-config/src/lib.rs:250-261.
   • Validation does not reject irrelevant path for non-file outputs:
     • Output validation switch: crates/logfwd-config/src/lib.rs:693-760
     • Stdout | Null branch is empty: crates/logfwd-config/src/lib.rs:748
     • Endpoint-based outputs validate endpoint/compression/request_mode, but not extraneous path: crates/logfwd-config/src/lib.rs:694-739
   • Runtime sink construction ignores cfg.path for most non-file outputs:
     • Matcher entrypoint: crates/logfwd-output/src/lib.rs:662-790
     • stdout path unused: crates/logfwd-output/src/lib.rs:769-776
     • otlp path unused: crates/logfwd-output/src/lib.rs:740-767
   • Exception: Elasticsearch uses path as fallback for index:
     • crates/logfwd-output/src/lib.rs:668-673

   Relevant snippet:

   // crates/logfwd-output/src/lib.rs:668-673
   let index = cfg
       .index
       .as_ref()
       .or(cfg.path.as_ref())
       .map_or("logs", String::as_str)
       .to_string();

   Verification

   I ran these commands locally in the workspace:

   RUSTC_WRAPPER= cargo build -p logfwd --quiet
   ./target/debug/logfwd --config /tmp/gh-aw/agent/issue1066-stdout-path.yaml --validate
   ./target/debug/logfwd --config /tmp/gh-aw/agent/issue1066-otlp-path.yaml --validate

   Observed output:

   --- validate stdout+path ---
     ready: default
   config ok: 1 pipeline(s)
   --- validate otlp+path ---
     ready: default
   config ok: 1 pipeline(s)
   

   So both configs currently pass validation.

   Detailed Action Plan

   1. In crates/logfwd-config/src/lib.rs inside Config::validate() output loop (for (i, output) around :676+), add a pre-check after unsupported output-type checks:
      • If output.path.is_some() and output type is one of {Stdout, Null, Otlp, Http, Loki, Tcp, Udp, ArrowIpc}, return ConfigError::Validation.
   2. Decide Elasticsearch behavior explicitly:
      • Option A (strict): also reject path for Elasticsearch and require index only; remove .or(cfg.path.as_ref()) in crates/logfwd-output/src/lib.rs:671.
      • Option B (compat): keep alias and add a config test + docs note that path is legacy alias for index only for Elasticsearch.
   3. Add table-driven tests in crates/logfwd-config/src/lib.rs test module validating rejection message for each non-supported output type with path set.
   4. Add one positive Elasticsearch test matching whichever option is selected.

   Related Items

   Type	Link	Relevance
   Issue	#1066	Current bug report
   Issue	#530	Prior validation/runtime mismatch for output config
   Issue	#718	Prior output validation inconsistency
   PR	#795	Recent output validation changes tied to #530/#718
   Issue	#1060	Similar class: stdout accepts invalid output-side field values
   File	crates/logfwd-config/src/lib.rs	Validation logic to update
   File	crates/logfwd-output/src/lib.rs	Runtime sink behavior (path usage/ignore)

   Note

   🔒 Integrity filtering filtered 4 items

   Integrity filtering activated and filtered the following items during workflow execution.
   This happens when a tool call accesses a resource that does not meet the required integrity or secrecy level of the workflow.

   • issue:bug: non-file outputs accept output.path and ignore it silently #1066 (issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)
   • issue:bug: README shows elasticsearch as implemented but --validate rejects it as 'not yet implemented' #718 (issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)
   • issue:file_out output type passes --validate but crashes at pipeline construction #530 (issue_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)
   • pr:fix: config validation for elasticsearch, loki, file_out (#718, #530) #795 (pull_request_read: has lower integrity than agent requires. The agent cannot read data with integrity below "approved".)

   ---

   What is this? | From workflow: Issue Triage

   Give us feedback! React with 🚀 if perfect, 👍 if helpful, 👎 if not.
3. strawgate commented on Apr 5, 2026

   

   strawgate

   on Apr 5, 2026

   OwnerAuthor

   More actions

   Validation: CONFIRMED — shared root cause with all silent-ignore config issues

   Both InputConfig and OutputConfig are single flat structs (#[serde(deny_unknown_fields)]) with fields shared across all input/output types. Config::validate() checks that required fields are present for each type but does not reject extraneous optional fields that have no meaning for the selected type.

   This bug is one of ~12 confirmed instances of the same pattern (see also #1057, #1058, #1060, #1064, #1065, #1066, #1070, #1071, #1072, #1073, #1075, #1077, #1078).

   Root cause confirmed:

   • crates/logfwd-output/src/lib.rs: runtime builders use match cfg.output_type and silently ignore fields not consumed by the selected arm.
   • crates/logfwd-config/src/lib.rs Config::validate(): validates required fields per type but has no reject-if-present checks for inapplicable optional fields.

   Unified fix — add per-type field rejection to Config::validate():

   // Example for OutputConfig:
   match output.output_type {
       OutputType::Stdout | OutputType::Null => {
           if output.endpoint.is_some() { warn_or_err("endpoint not used by this output type"); }
           if output.index.is_some() { warn_or_err("index not used by this output type"); }
           if output.compression.is_some() { warn_or_err("compression not used by this output type"); }
           if output.auth.is_some() { warn_or_err("auth not used by this output type"); }
           // etc.
       }
       OutputType::Elasticsearch => {
           if output.protocol.is_some() { warn_or_err("protocol not used by elasticsearch"); }
           // etc.
       }
       // ...
   }

   A single pass over all output types covering all optional fields fixes all 12 related issues at once.
4. strawgate

   added 3 commits that reference this issue on Apr 5, 2026

   fix(config): reject type-inappropriate fields in input/output validation

   ...

   5dfde4a

   fix(config): reject type-inappropriate fields in input/output validation

   ...

   802bdb3

   fix(config): reject type-inappropriate fields in input/output validation

   ...

   2d249da
5. strawgate commented on Apr 5, 2026

   

   strawgate

   on Apr 5, 2026

   OwnerAuthor

   More actions

   Fixed on master in PR #1123 (923a033bd8e855f4011f8ad9d5248b9e44a3546a).

   I rechecked origin/master; these type-inappropriate config fields are now rejected during validation, so this issue/work-unit looks stale and safe to close.
6. strawgate

   closed this as completedon Apr 5, 2026