Repository navigation
fix: redact credential-like fields in meta before logging - #108
Open
anrenlx2025 wants to merge 1 commit into
Open
anrenlx2025 wants to merge 1 commit into
anrenlx2025 wants to merge 1 commit into
Conversation
The frontier server logs the full connection meta (edgebound/servicebound/ exchange, 24 call sites) on edge/service online/offline, heartbeat, stream and forward paths. Clients commonly carry credentials (e.g. access_key/ secret_key) in meta, so every connect/disconnect cycle writes plaintext credentials into system logs. Add misc.Redact(meta string) string and wrap the meta log sites: - Parse the JSON meta and replace the value of any key whose lowercased name contains a credential-like fragment (key/secret/token/password/ passwd/pass/pwd/credential/auth/signature/bearer/session/cookie/cert/ private) with "***", recursively (nested objects, objects inside arrays). Non-string values under sensitive keys are also masked. Substring matching intentionally errs on the side of over-masking: innocuous keys such as "keyword" or "author" are masked too, which is preferred over leaking. - Fail-closed: non-JSON input, oversized input (>8KB), and JSON whose top-level value is not an object/array all return a placeholder without echoing the raw value. A top-level string often carries an already-serialized object, so it is masked as a whole. - Wire the geminio SDK end logger through klog (SetLog) for edgebound and servicebound ends, mirroring the existing frontlas wiring: the SDK default logger prints raw connection meta to stdout on close-handshake error paths, which would bypass the redaction above. - Table-driven tests cover masking variants, case-insensitivity, nesting, non-string sensitive values, non-JSON, double-encoded JSON, top-level scalars, trailing garbage, over-redaction acceptance, and the exact 8KB boundary. Known boundaries (intentionally out of scope): meta stored in the repo, returned by control-plane APIs, or forwarded to frontlas is functional data flow, not logging; unchanged. meta.Service in the service forward path is a plain routing name parsed from validated JSON, not a credential carrier, and is logged as is.
|
Someone is attempting to deploy a commit to the singchia's projects Team on Vercel. A member of the Team first needs to authorize it. |
Author
|
Thanks for reviewing when you get a chance. Since this is my first contribution to this repo, the CI workflow on the fork PR needs maintainer approval to run ( |
This branch has not been deployed
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.
Problem
The frontier server logs the full connection meta (edgebound/servicebound/exchange, 24 call sites) on edge/service online/offline, heartbeat, stream and forward paths. Clients commonly carry credentials (e.g.
access_key/secret_key) in meta, so every connect/disconnect cycle writes plaintext credentials into system logs. Several of these sites areklog.Errorf(unconditional), the rest areklog.V(1..3). In addition, the geminio SDK's default logger prints raw meta to stdout on close-handshake error paths, bypassing klog entirely.Fix
Add
misc.Redact(meta string) string, wrap all 24 meta log sites, and close the SDK logging bypass:key/secret/token/password/passwd/pass/pwd/credential/auth/signature/bearer/session/cookie/cert/private) with***, recursively (nested objects, objects inside arrays). Non-string values under sensitive keys are also masked. Substring matching intentionally errs on the side of over-masking: innocuous keys such askeywordorauthorare masked too, which is preferred over leaking.<redacted: unparsable meta>; input larger than 8KB returns<redacted: meta too long>; JSON whose top-level value is not an object/array (e.g. a bare string, which often carries an already-serialized object) returns<redacted: non-object meta>. Raw meta is never echoed.opt.SetLog(log.NewKLog())) for edgebound and servicebound ends, mirroring the existing wiring infrontlas/frontierbound: with the default logger, the SDK prints raw connection meta to stdout on close-handshake error paths, which would bypass the redaction above.Known boundaries (intentionally out of scope)
meta.Servicein the service forward path is a plain routing name parsed from validated JSON, not a credential carrier; logged as is.Redactruns even when klog verbosity filters the line out (argument eager evaluation); measured cost (~10-90us for typical <1KB to 8KB meta) is negligible for typical fleet sizes, kept simple rather than wrapping 24 sites inklog.V(n).Enabled()guards.Test plan
go test ./pkg/frontier/misc/(table-driven tests, 24 cases)go build ./...on both windows and linux,go veton touched packagesgofmtblob-level clean on all 9 changed filespkg/frontier/edgeboundTestEdgeManagerStreampanics atnewEdgeManageron both v1.2.5 baseline and this branch (unrelated to this change, reproduced via stash)