feat: add redacted FALCON_DEBUG mode for bash and PowerShell - #522
Conversation
f06f83d to
023cc3c
Compare
carlosmmatos-cs
left a comment
There was a problem hiding this comment.
Reviewed the debug feature closely, and I like the shape of it — the allowlist discipline at the call sites is genuinely good and the credential protections from #520 are untouched. Two things block merging, though, and the first one is the important one.
1. The branch is stacked on the wrong commit and will revert #521
#521 is already merged (dad81bf, squash). This branch is stacked on 56a3cef, which is an intermediate commit of #521, not its final tip (d2e05e7). #521 gained three more commits after 56a3cef:
0354785re-issue the OAuth token request instead of following the redirect6b54ff1drop the dead redirect pin fromfetch_tags6a2ae59correct the header-collection note inGet-FalconRegionHeader
None of those are in this branch, so merging as-is reverts them. Diffing this branch against origin/main shows the regressions in both languages:
bash — main has an oauth_token_request() helper plus a region-retry block that re-issues the token request against the x-cs-region hint. This branch removes both and goes straight to die when the token comes back empty:
# main (bash/install/falcon-linux-install.sh)
hinted=$(grep -i ^x-cs-region: "${response_headers}" | ...)
if [ -n "$hinted" ] && [ "$hinted" != "$cs_falcon_cloud" ]; then
retry_host=$(cs_cloud "$hinted")
...
token_result=$(echo "$auth_payload" | oauth_token_request "$retry_host" "$retry_headers")
# this branch
if [ -z "$token" ]; then
die "Unable to obtain CrowdStrike Falcon OAuth Token. ..."
fi
PowerShell — main has Get-FalconRegionHeader(), added precisely because the response-header collection type differs by platform. This branch drops it and returns to the old single-form access:
# this branch, falcon_windows_install.ps1:285
if ($response.Headers.Contains('X-Cs-Region')) {
$region = $response.Headers.GetValues('X-Cs-Region')[0]So both halves of "keep region auto-discovery" — the thing #521's title promises — get undone. A customer who leaves FALCON_CLOUD unset, or sets the wrong region, goes from a working auto-discovery retry to a hard failure.
For the record, a rebase is not mechanical here. git rebase origin/main conflicts in all 7 script files on the very first commit, and cherry-picking only the debug commit (023cc3c) onto main also conflicts in all 7. Please rebase onto origin/main, drop 4fd02b1 / 3990cf5 / 56a3cef entirely, and keep only the debug commit. After that the diff should be debug-only, as the PR description intends.
2. FALCON_DEBUG=1 breaks the sensor download and dumps the package to stdout
The debug branch in curl_command inserts its own -o "$body_file" before "$@":
http_code=$(printf '%s\n' "$auth_config" |
curl -s -x "$proxy" -L --proto '=https' --proto-redir '=https' -K- \
-o "$body_file" -w '%{http_code}' "$@") || curl_rc=$?
...
cat "$body_file"But download_installer supplies its own -o:
bash/install/falcon-linux-install.sh:468
curl_command "https://$(cs_cloud)/sensors/entities/download-installer/v3?id=$sha" -o "${installer}"
curl binds output files to URLs positionally, so with one URL and two -o the first one wins. I confirmed the precedence directly:
$ curl -s -o /tmp/o1 -w '%{http_code}' https://example.com -o /tmp/o2
http_code=200
o1 size: 559
o2: never created
And reproduced the end state with the actual function body:
### calling curl_command with caller-supplied -o (as download_installer does)
--- installer file created? ---
NO -- installer MISSING (install would fail)
--- stdout captured ---
stdout bytes: 559
<!doctype html><html lang="en">...
So with debug on, the sensor package lands in the mktemp file instead of ${installer}, ${installer} is never created, and cat "$body_file" writes the whole package to stdout. The installer is tens of megabytes, so the practical result is a broken install and a terminal full of binary — under exactly the flag a support engineer is asked to turn on.
Same code path in bash/migrate/falcon-linux-migrate.sh:280 with the caller at :760. falcon-container-sensor-pull.sh:325 carries the same branch but no caller passes -o, so it is unaffected today; it would be worth making all three consistent anyway.
Simplest fix is to keep the caller's own output handling and get the status a different way — for example -w '%{http_code}' written to a separate file with --stderr, or --write-out to a fd, rather than injecting -o. Whatever the approach, please add a debug-on regression check for download_installer; the current test plan exercises the auth paths but not the download path, which is why this got through.
3. The scrub is a fail-open denylist that misses this repo's own variable names
Not a live leak — I enumerated every falcon_debug / Write-FalconDebug call site across all 7 scripts and none of them interpolate a secret. They pass step names, cloud, region hint, HTTP status, curl exit, and SSM parameter name. That part is well done, and the catch blocks deliberately take only [int]$_.Exception.Response.StatusCode instead of Exception.Message, which is the right call.
The concern is that the scrub is presented as the safety net, and it only fires when a recognised label immediately precedes the value:
client_secret=SUPERSECRETVALUE123 -> client_secret=[REDACTED]
FALCON_CLIENT_SECRET=SUPERSECRETVALUE123 -> FALCON_CLIENT_SECRET=SUPERSECRETVALUE123
cs_falcon_oauth_token=SUPERSECRETVALUE123 -> cs_falcon_oauth_token=SUPERSECRETVALUE123
ART_PASSWORD=SUPERSECRETVALUE123 -> ART_PASSWORD=SUPERSECRETVALUE123
X-aws-ec2-metadata-token: SUPERSECRET... -> X-aws-ec2-metadata-token: SUPERSECRET...
SUPERSECRETVALUE123 -> SUPERSECRETVALUE123
[Cc]lient_[Ss]ecret cannot match CLIENT_SECRET, so the all-caps env var name slips through even though OLD_/NEW_FALCON_CLIENT_SECRET got explicit all-caps patterns. So today's safety rests on call-site discipline, not on the scrub, and the next person to add a falcon_debug line will not get caught by it. Adding the actual variable names in use — FALCON_CLIENT_SECRET, cs_falcon_oauth_token, ART_PASSWORD, X-aws-ec2-metadata-token, aws_secret_access_key — would make the net match the claim.
4. Small things
- The READMEs say
Accepted values are ['1', 'true'], but bash accepts1|true|TRUE|yes|YES|on|ONand PowerShell matches^(1|true|yes|on)$case-insensitively. Docs and code should agree. Authorization: Bearer $tokenscrubs toAuthorization:[REDACTED] [REDACTED]because two patterns both fire. Harmless, just untidy output.mktempfailure is unchecked; if it fails,body_fileis empty and curl gets-o "".- The PowerShell README says tracing "is forced off at startup (
Set-PSDebug -Off) so credentials stay out of logs".Set-PSDebug -Offis pre-existing and does work, but it runs insidebegin{}— after parameter binding. Someone who runsSet-PSDebug -Trace 2before invoking the script still gets the binding of-FalconClientSecrettraced. Worth softening the wording so it does not over-promise.
What checks out
- shfmt (
-i 4 -ci, both-ln bashand-ln posix) and shellcheck (bash and dash) are clean on all 4 bash scripts — 16/16 checks, no output. - The
set +xguard from #520 is intact at the top of all 4 scripts, before any credential handling. - Nothing re-enables tracing: no
set -x, noSet-PSDebug -Trace, no$VerbosePreference/$DebugPreferenceassignment, noStart-Transcript, and no-Verbose/-vadded to any curl orInvoke-WebRequestcall. - No debug code touches the
set -- "$@"shift no-op from #520, and no secret is added to curl argv — the token still goes through-K-on stdin. Write-FalconDebugusesWrite-Host, so it stays out of the pipeline and cannot corrupt a captured return value. It also does not route throughWrite-FalconLog, so debug lines do not persist to the temp log file. Both are the right choices.--debugparsing is consistent with each script's existing style, and--helpstill works.
Happy to re-review once it is rebased on origin/main and the -o collision is sorted.
09b958e to
d5ee929
Compare
d5ee929 to
89c9bc3
Compare
Support currently has to reach for `bash -x` or `Set-PSDebug -Trace` to diagnose a failing install, and both print credentials. This adds an opt-in debug mode that answers the questions support actually asks, without ever printing a value that is not known to be safe. bash gets `FALCON_DEBUG=1` or `--debug`; PowerShell gets `-FalconDebug` or `$env:FALCON_DEBUG`. Markers cover the detected OS, architecture, kernel and package manager; the exact FQL sensor query filter and how many installers matched; which installer was selected, its size and SHA-256 check; the API route, HTTP status and curl exit code for every call; the resolved sensor update policy version; and the installed sensor version and AID. The filter is the point of the whole feature. "No sensor found for OS" and "why did it pick that version" are unanswerable without seeing the query that was sent, and it is assembled from four separately derived pieces. Redaction is fail-closed. Only keys on a fixed allow-list keep their value; everything else becomes `[DROPPED]`, and a bare token with no key is dropped entirely. Anything user-supplied that could hold a secret is reported as presence only, under a distinct `_set` key — `provisioning_token` is not allow-listed, so a future line that prints it still drops. Customer data is reported as counts, not contents. In PowerShell, values that can contain a space are passed through `-Pairs` rather than a joined string. An FQL filter can hold a multi-word policy name, and any scheme that splits a joined string would either mangle it or, worse, glue a bare word onto an allow-listed value and print it. Debug adds no request of its own. `curl_command` already dumps headers and buffers the body, so the marker reports the status and exit code it has computed; the curl invocation is untouched. The exit code is reported inside `handle_curl_error`, which receives it on every failure path, so no call site captures `$?` and none of the `||` chains change. Neither language re-enables tracing. The `set +x` guard and `Set-PSDebug -Off` both stay, and no curl or Invoke-WebRequest gains a verbose flag. The PowerShell catch blocks now record a status code instead of serializing the whole exception object into the on-disk log. READMEs document the new mode with sample output from real runs, and stop recommending `bash -x` and `Set-PSDebug -Trace` for support.
89c9bc3 to
a0311fc
Compare
Bumps the version string from 1.13.0 to 1.14.0 across all scripts and READMEs ahead of the v1.14.0 release. This is a minor bump because of the new opt-in FALCON_DEBUG mode added to the bash and PowerShell scripts (#522), which ships alongside four fixes: the credential protections in the deployment scripts (#520), the OAuth and fetch_tags redirect hardening that keeps region auto-discovery (#521), the curl_command fix that stops the bearer token crossing a redirect (#525), and the handle_curl_error path under sh (#526). Updates the VERSION and $ScriptVersion constants in the bash and PowerShell scripts, the Version usage lines in the READMEs, the pinned raw.githubusercontent.com URLs, and the FALCON_DEBUG sample output.
Summary
Support currently has to reach for
bash -xorSet-PSDebug -Traceto diagnose a failing install, and both print credentials. This adds an opt-in debug mode that answers the questions support actually asks, without ever printing a value that is not known to be safe.FALCON_DEBUG=1or--debug-FalconDebugor$env:FALCON_DEBUGReal output from a live install on Ubuntu 22.04 against
us-2:The
filter=line is the point of the whole feature. "No sensor found for OS" and "why did it pick that version" are unanswerable without seeing the query that was sent, and it is assembled from four separately derived pieces.path=distinguishes the four API calls that previously all logged as an identicalhttp_status=200 curl_exit=0.aid=noneimmediately after install is correct, not a bug: registration completes asynchronously once the sensor reaches the cloud. A later uninstall run on the same host showedaid=cac191224a994075ad2fbb0a05139807.Redaction is fail-closed
Only keys on a fixed allow-list keep their value; everything else becomes
[DROPPED], and a bare token with no key is dropped entirely:Two details in there are load-bearing rather than cosmetic:
Credentials are reported as presence only, under a distinct
_setkey.provisioning_token_set=yesis allow-listed;provisioning_tokendeliberately is not. So a future line that writesprovisioning_token=$SECRETstill drops. Allow-listing the credential's own name would have made exactly that mistake print the secret. Customer data is reported as counts (tags_count=3), never contents.PowerShell passes space-bearing values through
-Pairs [ordered]@{}instead of a joined string. An FQL filter can hold a multi-word sensor update policy name (name.raw:'My Update Policy'), and every string-splitting scheme fails on it: splitting on whitespace mangles the filter, and re-joining continuation tokens is fail-open —step=x SECRETwould printSECRET. So the value has to arrive out-of-band from its key. Both the-Messageand-Pairspaths funnel through oneProtect-FalconDebugPair, so they cannot diverge.Debug adds no request of its own
After #525,
curl_commandalready dumps headers and buffers the body, so the marker reports the status and exit code it has already computed — the curl invocation is untouched, and there is no second copy that could drift from the security path. The exit code is reported insidehandle_curl_error, which receives it on every failure path, so no call site captures$?and none of #526's|| handle_curl_error $?chains change.Neither language re-enables tracing. The
set +xguard andSet-PSDebug -Offboth stay, and no curl orInvoke-WebRequestgains a verbose flag. The PowerShell catch blocks record a status code instead of serializing the whole exception object into the on-disk log — that removes 5 of the 6 pre-existingWrite-VerboseLog -VerboseInput $_.Exceptioncalls in migrate, and this PR removes the sixth.Rebased on main
Rebuilt on
29b737f, so #520, #521, #525 and #526 are all included. Verified by count rather than by eye: every security invariant from those PRs (oauth_token_request,x-cs-region,retry_host,--proto,-K-,set +x,|| handle_curl_error $?,-MaximumRedirection,$RedirectResponse,Get-FalconRegionHeader,Get-FalconCloud,Set-PSDebug -Off) occurs exactly as often as onmain, and-Loccurs zero times in all four bash scripts.Verification
All 6 CI checks green: bash, dash, shfmt, PSScriptAnalyzer, powershell, Broken Links. Locally, 16/16 (shfmt
-ln bashand-ln posix, shellcheck bash and dash, per file) are silent.Run live against the real API, not just linted:
source=api maintenance_token_set=yesandexit_code=0.61c06e35b310…) and version8.10.21405.access_token|maintenance-tokengrep were the_set=nopresence flags.-o, under bash and dash, debug on and off: the installer is written every time, stdout carries 0 bytes, the body is intact when the caller captures it instead, no marker text reaches the body, and debug on/off produce identical file output. This is the check that did not exist before and is why the earlier-ocollision shipped.provisioning_token/maintenance_token/FALCON_CLIENT_SECRET/ an unknown future key / a bare value all drop, and a filter containing a multi-word policy name survives intact.FALCON_DEBUG=yescorrectly does nothing, matching the documented1|truecontract.--helpand-hwork in any argument position on all four scripts, under both shells.One practical note for support, learned the hard way while testing: the scripts print progress with
echo -n, so a marker can share a line with a progress message. Match them withgrep FALCON_DEBUG, notgrep '^FALCON_DEBUG'.