Skip to content

fix: stop camera credentials leaking through logs and the monitor settings UI - #308

Merged
pliablepixels merged 4 commits into
mainfrom
fix/307-credential-redaction
Jul 29, 2026
Merged

fix: stop camera credentials leaking through logs and the monitor settings UI#308
pliablepixels merged 4 commits into
mainfrom
fix/307-credential-redaction

Conversation

@pliablepixels

Copy link
Copy Markdown
Member

Closes #307 once the maintainer confirms.

A user testing with an unprivileged ZoneMinder account found that the client receives and displays every camera's connection settings, credentials included. The API side is upstream (ZoneMinder has no per-field ACL on monitors.json), but the client was making it worse: log redaction did not recognize the shape a camera password takes, and the settings dialog put it on screen behind a reveal button.

Logs

Each of these was verified by probing the real sanitizeObject/logger with redaction on, not by reading the code.

  • Non-http schemes bypassed every URL rule. sanitizeUrl fires only on startsWith('http://'|'https://'), sanitizeLogMessage matches only /https?:\/\//. Monitor.Path is rtsp://admin:secret@cam/live. New lib/security/url-credentials.ts owns one scheme-agnostic regex, applied to every string the sanitizer sees, at any position, so Monitor.Options (-i rtsp://admin:secret@cam/live) is covered too.
  • The two copies of the string rules in log-sanitizer.ts collapse into one sanitizeString. They had drifted; that is what let a value-position credential through while a message-position one was caught. URL handling now runs before form-data handling, so a query string with a password stays a readable URL instead of being percent-encoded into one blob.
  • ERROR-level component logs flattened details with JSON.stringify before sanitizing. The sanitizer redacts by key and a flattened object has none, so every secret in an ERROR detail survived, across ~130 call sites. Details are passed as objects at every level now.
  • cookie and credential join the sensitive keys. ZMSESSID was riding out in logged request and response headers.
  • A sensitive key holding an object is recursed into rather than stringified to '[obje...'; a single-field form body (pass=secret) no longer needs an & to be recognized; console.error(error.stack) is sanitized, since the stack embeds the message where the URL usually is.
  • ZoneMinder's own log lines were not sanitized at all. The Server tab mapped zmLog.Message straight to the display entry, and Save/Share export what is on screen. zmc logs the ffmpeg command line it runs. That is the file users attach to bug reports.

Reachability, for severity: the HTTP client logs response bodies, per-component levels are user-settable, and the HTTP failure branch logs at ERROR, which clears the default INFO level with no opt-in at all.

UI

Decided with the maintainer:

  • The monitor Pass field keeps its row and stays editable, but loses the reveal-eye toggle while redaction is on.
  • Path displays with the password segment masked; host, port, and stream path stay readable. restoreUrlCredentials puts the real password back on save, so changing a hostname does not wipe a password the user cannot see. Typing over the mask makes what was typed the new password.
  • Other password inputs (profile password, Ollama API key, kiosk PIN) are unchanged: those are secrets the user typed on their own device, where hiding them mostly blocks typo-checking.

Not in scope

The assistant already allowlists monitor fields in mapMonitor, so no credential reaches a model. And none of this substitutes for the upstream problem: any account that can view a monitor can curl the credentials directly. Both user docs say so.

Verification

npm run gates passes: 3038 tests, build, three blocking lints within baseline. 30 new tests across url-credentials, log-sanitizer, logger-error-details, Logs, and MonitorSettingsDialog; each was watched failing before the fix landed.

No e2e scenario: the masking outcome depends on a monitor whose stored path actually contains a credential, which the live test server does not guarantee. The component tests assert the real outcomes instead (rendered value, and the exact save payload for both the edited-host and typed-over-mask cases).

Docs: user-guide/logs.md gains a "What gets redacted" section, user-guide/monitors.md a "Camera Credentials" section, developer-guide/12 covers the new module and the pass-details-as-objects rule. agents/project/domain-context.md records the API behavior per M5.

Claude assisting @pliablepixels

🤖 Generated with Claude Code

pliablepixels and others added 3 commits July 28, 2026 17:58
…structured

A camera's password lives in the monitor's source path as URL userinfo
(rtsp://admin:secret@cam/live). Redaction never found it: sanitizeUrl fires
only on strings starting http:// or https://, and sanitizeLogMessage matches
only /https?:\/\//, so rtsp, rtmp, and anything mid-string passed through
verbatim. The HTTP client logs response bodies, and its failure branch logs
at ERROR, which clears the default level with no opt-in, so a failed
monitors.json request wrote every camera credential to the log file.

lib/security/url-credentials.ts owns the one regex that knows where a
password sits in a URL, whatever the scheme, wherever in the string. The two
copies of the string rules in log-sanitizer collapse into one sanitizeString
that runs it first. URL handling now precedes form-data handling, so a query
string with a password in it stays a readable URL instead of being
percent-encoded into one blob.

The ERROR path serialized its details with JSON.stringify before sanitizing.
The sanitizer redacts by key, and a flattened object has no keys, so every
secret in an ERROR detail survived across ~130 call sites. Details are
passed as objects at every level now; both sinks already pretty-print them.

Also: cookie and credential join the sensitive keys (ZMSESSID rode out in
logged headers), a sensitive key holding an object is recursed into rather
than stringified to '[obje...', a single-field form body no longer needs an
'&' to be recognized, and console.error(error.stack) is sanitized, since the
stack embeds the message where the URL usually is.

Refs #307

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Server tab maps zmLog.Message straight into the display entry, and the
Save and Share buttons export whatever is on screen. zmc and zma log the
ffmpeg command line they run, which carries the camera credential, so the
log file users attach to a bug report handed out every camera password on
the server.

Same sanitizeLogMessage the app's own entries go through, applied where the
server's entries enter the view.

Refs #307

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ZoneMinder returns Path, User, and Pass to any account that can view a
monitor, and pre-1.38 servers have nowhere but the source path to keep a
camera password. The dialog rendered both verbatim, with a reveal toggle on
the password field, so the credentials for every camera were one tap and one
screenshot away.

While log redaction is on, the source path shows with its password segment
masked and the Pass field loses its reveal button. Host, port, and stream
path stay readable: that is what makes the field worth showing.

Both fields stay editable. maskUrlCredentials produces what is on screen and
restoreUrlCredentials puts the real password back on save, so changing a
camera's hostname does not silently wipe a password the user cannot see.
Typing over the mask means the new value is the new password.

The same helpers back the log sanitizer, so what the UI hides and what the
logs hide cannot drift.

Refs #307

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pliablepixels pliablepixels added the core Changes core behavior; full review ceremony label Jul 28, 2026
PipActivity logged the URL it was handed and the raw media3 playback error.
The URL is an event video URL, which carries the ZoneMinder access token as
a query parameter, and a media3 HTTP failure quotes the URL it failed on
with the cause chain repeating it. Both went to logcat in cleartext, where
any adb session or bug report picks them up.

The onCreate line logs whether a URL arrived rather than the URL. The error
line logs the error code name plus a message with query strings stripped,
and drops the throwable, whose cause chain reprinted the URL.

Not covered by the JS sanitizer: this is native logging, on the other side
of the bridge from lib/logger.ts. No gate reaches it, so it is prose in
agents/project/native.md rather than a lint rule.

Refs #307

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@pliablepixels

Copy link
Copy Markdown
Member Author

Follow-up after checking the other two log sinks.

Capacitor log file: covered. CapacitorLogFileStore.append writes the LogEntry the logger already sanitized, so the NDJSON on disk and the text rendered on Share carry the redacted values. The one raw call in that file is console.warn('[log-file] append failed', err), a Filesystem error with no request data in it.

JS HTTP logs: covered. Probed the real log.groupCollapsed('HTTP', ...) path with a monitors.json body shaped like the reporter's:

url:     https://[REDACTED]/zm/api/monitors.json?token=eyJhb...
headers: Authorization "Basic...", Cookie "ZMSES..."
body:    user=admin&pass=%5BREDACTED%5D
Monitor: Path "rtsp://admin:[REDACTED]@192.168.1.9:554/h264"
         Pass "[REDACTED]", ONVIF_Password "[REDACTED]"
         Options "-rtsp_transport tcp -i rtsp://admin:[REDACTED]@cam/live"

Everything sensitive is gone and the line stays readable.

Native logging: was not covered, now fixed. The sanitizer lives on the JS side of the bridge and nothing native goes through it. PipActivity logged the URL it was handed, which is an event video URL built by getEventVideoUrl with accessToken stamped into the query, and it logged the raw media3 PlaybackException whose message quotes the failing URL with the cause chain repeating it. Both landed in logcat in cleartext, reachable from any adb session or bug report.

onCreate now logs hasUrl= instead of the URL. The error line logs getErrorCodeName() plus a query-stripped message and drops the throwable. Verified with ./gradlew :app:compileDebugJavaWithJavac.

No gate reaches native logging, so the rule went into agents/project/native.md as prose rather than a lint rule. Swept the rest of the native sources: the only other NSLog/CAPLog.print/Log.* calls are in LlamaEngine.swift, AppleIntelligencePlugin.swift, and the remaining PipActivity lines, none of which touch a URL or a credential.

npm run gates still passes: 3038 tests, build, three lints within baseline.

Claude assisting @pliablepixels

@pliablepixels
pliablepixels merged commit 17c1075 into main Jul 29, 2026
8 checks passed
@pliablepixels
pliablepixels deleted the fix/307-credential-redaction branch July 29, 2026 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Changes core behavior; full review ceremony

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Extra checks to remove credentials in logs including those that come from ZM

1 participant