Skip to content

logs-server CLI: send config banner to stderr + investigate shell:true deprecation (security) #50

Description

@DawidWraga

Context

From a real diagnosis session using logs-server end-to-end: every logs-server query (and check) invocation prints two lines to stdout before any actual output:

(node:NNNN) [DEP0190] DeprecationWarning: Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.
[logs-server] loaded config from /Users/.../path/to/.davstack/config/logs-server.config.ts

Both are real problems — one operational, one potentially security-relevant.

Problem 1 — Banner pollution breaks pipes

Both lines go to stdout, not stderr. So logs-server query filter --json | jq '.[0]' fails parsing (Expecting value: line 1 column 1). I worked around with tail -n +3 | jq ... but the offset depends on whether the deprecation warning fires (it does on Node 22; might not on others). Fragile.

The config-loaded banner also leaks the absolute path of the user's repo. Harmless in a one-off shell, less great if stdout ever gets captured into a CI log, support transcript, or shared screenshot.

Fix

Send both lines to stderr. Two lines of code:

- console.log('[logs-server] loaded config from ' + path)
+ console.error('[logs-server] loaded config from ' + path)

…and silence the deprecation warning at the source (see problem 2).

Problem 2 — shell: true deprecation warning (security)

Node's message:

Passing args to a child process with shell option true can lead to security vulnerabilities, as the arguments are not escaped, only concatenated.

This is DEP0190. The warning fires somewhere in the daemon-spawn or query-CLI plumbing — needs to be hunted down. Two outcomes:

  1. The args are safe (no user-controllable input flows into the spawn). In that case, switch to shell: false with an explicit args array, OR document why shell: true is required + how the args are constructed safely. Either way the warning goes away.

  2. The args are not safe. Then this is a real injection vector (a user-controlled --grep value, a run_id, a config path) → command injection. Needs a security-grade fix.

I don't know which it is without reading the call site, but the warning being there at all means it deserves a deliberate decision rather than passing through.

Fix

  • Find the child_process.spawn / exec call with shell: true.
  • Audit which args it takes and whether any are user-controllable.
  • Switch to shell: false with proper arg arrays where possible.
  • Document the decision in code.

Acceptance

  • logs-server <verb> produces clean stdout suitable for piping to jq / python -c 'json.load(...)'
  • No DEP0190 deprecation warning at startup
  • Brief note in code comment explaining the shell: choice for the relevant spawn site

Severity

The pipe-breakage is daily friction. The shell: true warning may or may not be a real security bug — won't know until someone investigates. Labelled security so it gets eyes; downgrade if the audit shows no user-controllable input.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingsecuritySecurity-relevant: investigate / fix

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions