Skip to content

fix: close CodeQL clear-text-logging alert (no API-key substrings)#22

Merged
askalf merged 1 commit intomainfrom
fix/codeql-clear-text-logging
Apr 25, 2026
Merged

fix: close CodeQL clear-text-logging alert (no API-key substrings)#22
askalf merged 1 commit intomainfrom
fix/codeql-clear-text-logging

Conversation

@askalf
Copy link
Copy Markdown
Owner

@askalf askalf commented Apr 25, 2026

Summary

First CodeQL alert against the public repo. High severity, js/clear-text-logging, sink at src/util/output.ts:8 (the success() logger). Closes the alert by removing the upstream substring exposure and breaking the dataflow on the second flagged path.

Two upstream paths flagged

1. Real (low-but-nonzero) info disclosure — src/auth.ts:90-91

hands auth status line emitted sk-ant-...XXXX (first 7 + last 4 chars of the stored API key):

const masked = config.apiKey.slice(0, 7) + '...' + config.apiKey.slice(-4);
output.success(`Mode: API Key (${masked})`);

The first 7 chars are the well-known fixed sk-ant- prefix (zero entropy), but the last 4 are real key material. Anthropic API keys are long-lived; emitting any substring to a place that ends up in screenshots / pasted bug reports / shared terminal scrollbacks widens the exposure surface for no operational benefit.

Fixed: display *** only. Matches dario v3.7.2+'s "no substring of any stored key in user-facing output" rule. The user-visible behavior is now Mode: API Key (***).

2. CodeQL false-positive (cleanly resolved) — src/init.ts:93

output.success(`auth: ${config.authMode}${config.authMode === 'api_key' && config.apiKey ? ' (key stored)' : ''}`);

The template's true-branch is the literal string ' (key stored)'config.apiKey's value is never emitted, only its truthy-ness is consumed. But CodeQL's js/clear-text-logging dataflow conservatively flags any access on a path that ends at a logger, so it tracks config.apiKey through the ternary into the template into success().

Fixed: route through a Boolean(...) intermediate so the flow stops there. Behavior unchanged — same string emitted in the same conditions.

const keyStored = config.authMode === 'api_key' && Boolean(config.apiKey);
output.success(`auth: ${config.authMode}${keyStored ? ' (key stored)' : ''}`);

Test plan

  • npm run typecheck — clean.
  • npm run build — clean.
  • npm test — 36/36 pass; no test referenced the masked format, so no test fix needed.
  • Required CI checks (actionlint, analyze / CodeQL, build (20/22)) on this PR.
  • CodeQL alert auto-dismisses after merge — analyze job re-runs on the merged commit, sees the dataflow is gone, and closes alert ci: foundation parity with dario / claude-bridge / deepdive #1.

First CodeQL alert against the public repo, high severity,
js/clear-text-logging on src/util/output.ts:8 (the success() sink).
Two upstream paths flagged via dataflow:

1. src/auth.ts:90-91 — `hands auth` status line emitted
   `sk-ant-...XXXX` (first 7 + last 4 chars of the stored API key).
   First 7 are the well-known fixed `sk-ant-` prefix (zero entropy),
   but the last 4 are real key material. Replaced with `***` only,
   matching dario v3.7.2+'s "no substring of any stored key in
   user-facing output" rule.

2. src/init.ts:93 — final summary line interpolated a literal
   ' (key stored)' based on a truthy check of config.apiKey. Value
   itself never emitted (template true-branch is a fixed string),
   but CodeQL's flow flags any access on the path to a logger.
   Routed through Boolean(...) so the dataflow stops there.

No behavior change beyond `hands auth` showing `Mode: API Key (***)`
instead of the partial-key string. Build clean, 36/36 tests pass,
no test referenced the masked format.
@askalf askalf merged commit 12d1256 into main Apr 25, 2026
4 checks passed
@askalf askalf deleted the fix/codeql-clear-text-logging branch April 25, 2026 23:55
askalf added a commit that referenced this pull request Apr 26, 2026
…ADME (#24)

Bundles three landed feature/fix PRs since v0.2.0 plus a structural
README rewrite. Additive — v0.2.0 Windows users see no behavior change.

Functional changes (already merged in earlier PRs, surfaced as v0.3.0):

- #22 — close CodeQL js/clear-text-logging high-severity alert; no
  substring of the stored API key emitted in user-facing output
  (matches dario v3.7.2+ rule).
- #23 — OS-aware system prompts. Both run modes branch on
  process.platform; macOS gets open + osascript guidance, Linux gets
  xdotool/ydotool with display-server detection. Pure-helper module
  src/system-prompt.ts with 13 unit-test assertions.

Documentation (this PR):

- README structural rewrite. Added sovereignty-angle lead, explicit
  cost-comparison table, full threat model with operating
  recommendations, honest Limitations & known issues block, FAQ,
  trust-and-transparency table mirroring claude-bridge. README size
  306 → 421 lines.
- CHANGELOG documents the README rewrite as a release-polish item.

Empirically un-smoked caveat (called out in the README's Limitations
section): the OS-branching is unit-tested but the LLM behavior under
the macOS / Linux blocks is not yet verified against a real model
call on a non-Windows host. First post-publish report from a Mac or
Linux user is the signal that locks in the cross-platform claim.

Version bump fires auto-release.yml on merge: tag, GitHub release,
inline npm publish --access public --provenance.

Co-authored-by: askalf <263217947+askalf@users.noreply.github.com>
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