Skip to content

fix: logger Error serialization, config validation on startup, dedup cleanup, sk-* secret masking#45

Open
Fengsh0923 wants to merge 1 commit intoop7418:mainfrom
Fengsh0923:fix/logger-config-validation
Open

fix: logger Error serialization, config validation on startup, dedup cleanup, sk-* secret masking#45
Fengsh0923 wants to merge 1 commit intoop7418:mainfrom
Fengsh0923:fix/logger-config-validation

Conversation

@Fengsh0923
Copy link
Copy Markdown

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.tssk-* API keys not masked in logs

OpenAI / Anthropic / third-party API keys (format: sk-<20+ chars>) were not covered by MASK_PATTERNS. Added:
```ts
/sk-[A-Za-z0-9]{20,}/g,
```

3. config.ts — No validation of required fields per channel

The daemon started silently even when required credentials were missing (e.g. CTI_FEISHU_APP_ID absent while feishu was in CTI_ENABLED_CHANNELS). The error only surfaced on the first user message with a cryptic SDK error.

Added validateConfig(config: Config): ConfigValidationError[] that checks:

  • At least one channel is enabled
  • Telegram: token + (chatId or allowedUsers)
  • Feishu: appId + appSecret
  • Discord: botToken + (allowedUsers or allowedChannels)
  • QQ: appId + appSecret

4. main.tscleanupExpiredDedup() existed but was never called

store.cleanupExpiredDedup() was implemented in JsonFileStore but never invoked, causing the dedup Map (and its backing dedup.json file) to grow without bound over time.

Added a 10-minute unref()'d interval so cleanup runs automatically without blocking shutdown.

main.ts also now calls validateConfig() at startup and exits with clear per-field error messages if config is invalid.

Test plan

  • Start daemon with a missing required field (e.g. remove CTI_FEISHU_APP_ID) — should exit immediately with a clear error message listing the missing field
  • Trigger an uncaught Error in the daemon — stack trace should now appear in bridge.log
  • Verify sk-xxx values are masked in log output
  • After many messages, confirm dedup.json file size stays bounded

…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant