fix: logger Error serialization, config validation on startup, dedup cleanup, sk-* secret masking#45
Open
Fengsh0923 wants to merge 1 commit intoop7418:mainfrom
Open
Conversation
…cret masking
- logger: serialize Error objects with stack trace instead of empty {}
(JSON.stringify(err) loses all Error fields; use err.stack ?? err.message)
- logger: add sk-* pattern to MASK_PATTERNS to cover OpenAI/Anthropic API keys
- config: add validateConfig() that checks required fields per enabled channel
(daemon previously started silently with missing credentials, only failing
on the first user message with a cryptic error)
- main: call validateConfig() on startup and exit(1) with clear field-by-field
error messages if required config is missing
- main: call store.cleanupExpiredDedup() every 10 min via unref'd interval
(the method existed but was never called, causing unbounded Map growth)
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
Four targeted bug fixes found during a code review:
1.
logger.ts— Error objects serialize to{}(loses stack trace)Before:
```ts
args.map((a) => (typeof a === 'string' ? a : JSON.stringify(a)))
```
JSON.stringify(new Error('boom'))returns'{}'— Error properties are non-enumerable. Stack traces are lost, making daemon errors very hard to debug.After:
```ts
args.map((a) => {
if (typeof a === 'string') return a;
if (a instanceof Error) return a.stack ?? a.message;
return JSON.stringify(a);
})
```
2.
logger.ts—sk-*API keys not masked in logsOpenAI / Anthropic / third-party API keys (format:
sk-<20+ chars>) were not covered byMASK_PATTERNS. Added:```ts
/sk-[A-Za-z0-9]{20,}/g,
```
3.
config.ts— No validation of required fields per channelThe daemon started silently even when required credentials were missing (e.g.
CTI_FEISHU_APP_IDabsent whilefeishuwas inCTI_ENABLED_CHANNELS). The error only surfaced on the first user message with a cryptic SDK error.Added
validateConfig(config: Config): ConfigValidationError[]that checks:4.
main.ts—cleanupExpiredDedup()existed but was never calledstore.cleanupExpiredDedup()was implemented inJsonFileStorebut never invoked, causing the dedup Map (and its backingdedup.jsonfile) to grow without bound over time.Added a 10-minute
unref()'d interval so cleanup runs automatically without blocking shutdown.main.tsalso now callsvalidateConfig()at startup and exits with clear per-field error messages if config is invalid.Test plan
CTI_FEISHU_APP_ID) — should exit immediately with a clear error message listing the missing fieldbridge.logsk-xxxvalues are masked in log outputdedup.jsonfile size stays bounded