mirror of
https://github.com/taylanbakircioglu/haproxy-openmanager.git
synced 2026-09-23 02:53:26 +00:00
taylanbakircioglu
190d45fe09
`POST /api/mfa/enroll` returns the new TOTP secret twice: once as `secret`,
which redaction already caught, and once inside `otpauth_uri` as a query
parameter, which it did not. Blanking one field while the same value sits three
keys away in cleartext is not redaction.
before: {"secret": "***REDACTED***",
"otpauth_uri": "otpauth://totp/OpenManager:admin?secret=JBSWY3DP..."}
after: {"secret": "***REDACTED***",
"otpauth_uri": "otpauth://totp/OpenManager:admin?secret=***REDACTED***"}
routers/mfa.py states the rule this restores: its own activity-log call records
`{"secret_len": len(secret_plain)}` with the comment "NEVER log the secret
itself".
The fix is not otpauth-specific. Any URI found in any captured string value now
goes through scrub_url, which was already written for exactly this and was only
ever pointed at the request line. That also covers:
* userinfo credentials - `https://user:pass@host/path` keeps only the host;
* any query-string credential in a body value or an error message, using the
same is_secret_key rules as the request line, so `?token=`, `?api_key=`,
`?password=` are all handled without naming them again here;
* fragments, which are dropped - they never reach a server and can carry
tokens.
Folded into the existing alternation rather than added as a second pass, so a
captured body is still scanned once. Two guards keep it from doing harm: a URI
with neither `?` nor `@` is returned untouched rather than rebuilt, and if
scrub_url reports a parse failure the original text is kept - inside a larger
string its placeholder would corrupt the surrounding sentence.
Verified: the TOTP secret and a userinfo password are removed, an in-text URL
inside an error message is scrubbed in place, and two innocent ACME URLs
(directory_url, account_url) come through byte-identical.
Also withdrawn here: the review flagged `POST /api/agents/generate-install-script`
as leaking the agent API key through its `script` field. That was wrong. The
finding was built from the endpoint's docstring EXAMPLE (routers/agent.py:497),
which shows an `api_key` field and an `API_KEY="agt_..."` line; the handler's
actual return is {script, platform, cluster_id, filename} and the template
substitution map has no token placeholder. The agent token reaches a node by a
different path entirely, and is issued as `api_key` (routers/security.py:144),
which REDACT_CONTAINS already covers. No change was needed and none is made.