Skip to content

fix(account): never log raw tokens, report only unexpected token failures - #11059

Merged
ArtyomSavchenko merged 2 commits into
hcengineering:developfrom
MichaelUray:fix/account-token-log-redaction
Oct 7, 2026
Merged

ArtyomSavchenko merged 2 commits into
hcengineering:developfrom
MichaelUray:fix/account-token-log-redaction

Conversation

@MichaelUray

Copy link
Copy Markdown
Contributor

Problem

getLoginInfoByToken and getLoginWithWorkspaceInfo log the full token whenever
it fails validation (ctx.error('Invalid token', { token })) and report every such
failure to Analytics.handleError. Expired or stale session tokens are routine, so
the error reporter receives expected failures and bearer credentials end up in log
storage. getWorkspacesInfo and updateLastVisit log the token the same way when
they reject a non-system caller. The logged error message is not safe either: the
token decoder passes the underlying library message through, and a decoder message
can quote the malformed input.

Change

  • tokenFingerprint(token) in server/account/src/utils.ts: 16 hex characters of
    an HMAC-SHA256 over the token, keyed with a key derived from the server secret.
    Logs keep a stable handle to correlate repeated failures of one token; the
    credential itself is never written, and the handle cannot be recomputed without
    the server secret. If no server secret is configured, no fingerprint is logged.
  • reportInvalidToken(ctx, token, err):
    • a TokenError (bad signature, expired, not yet active, revoked) is logged at
      warn level with the fingerprint and a fixed reason (a known decoder message,
      otherwise other) and is not reported to Analytics. This matches how wrap()
      already treats TokenError as an expected Unauthorized;
    • any other error is logged at error level with the fingerprint, the error class
      name and the reason unexpected, and is reported to Analytics as a fresh error
      that carries only the class name. The original error, its message, stack,
      cause and custom fields are never passed on.
  • The four call sites use the helpers. Status codes, the error mapping and the
    responses are unchanged. decodeTokenVerbose keeps logging the decoded claims
    of an unverifiable token, so operators lose no information.

Tests

  • utils.test.ts: fingerprint format, determinism, dependence on the server
    secret, no fingerprint without a secret, empty input; tokenErrorReason;
    reportInvalidToken for TokenError (warn, no Analytics) and for other errors
    (error, Analytics with a fresh error), with adversarial cases where the raw token
    is in the message, in cause, in a custom field, in the error name, or is part of
    a thrown string — asserting it appears in no log or Analytics call.
  • operations.test.ts: the server-token mock now exports the real TokenError and
    the plugin default (__esModule: true), Analytics is mocked. New cases for
    getLoginInfoByToken (expired token, decoder message quoting the token, non-token
    failure, no token), getLoginWithWorkspaceInfo and the caller checks of
    getWorkspacesInfo / updateLastVisit; the two existing token-error tests throw
    TokenError now.

…ures

getLoginInfoByToken and getLoginWithWorkspaceInfo logged the full token on
every validation failure and reported each failure to Analytics, although
expired or stale session tokens are routine. getWorkspacesInfo and
updateLastVisit logged the token when rejecting a non-system caller.

Log a keyed fingerprint instead of the credential (omitted when no server
secret is configured) together with a fixed failure reason. A TokenError is
logged at warn level and not reported to Analytics, as wrap() already
treats it. Any other error is logged at error level and reported as a
fresh error that carries only the error class name, never the original
message, stack, cause or custom fields. Status codes and responses are
unchanged.

Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
Comment thread server/account/src/utils.ts
@ArtyomSavchenko

Copy link
Copy Markdown
Member

Reference the mocked Analytics.handleError through one local jest.Mock
per test file instead of passing the unbound method to expect(), which
tripped @typescript-eslint/unbound-method in the formatting job.

Signed-off-by: Michael Uray <michaeluray@users.noreply.github.com>
@MichaelUray

Copy link
Copy Markdown
Contributor Author

@ArtyomSavchenko Fixed in c77b971: the new tests passed the unbound Analytics.handleError to expect(), which tripped @typescript-eslint/unbound-method. They now reference it through one local jest.Mock per file. rush fast-format --branch develop now passes with 0 errors and no diff, and the server/account tests pass.

@ArtyomSavchenko
ArtyomSavchenko merged commit 5077d97 into hcengineering:develop Oct 7, 2026
13 checks passed
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.

2 participants