Skip to content

fix(respect): secrets masking for encoded occurences in har-output - #2998

Open
DmitryAnansky wants to merge 7 commits into
mainfrom
fix/mask-encoded-secrets
Open

fix(respect): secrets masking for encoded occurences in har-output#2998
DmitryAnansky wants to merge 7 commits into
mainfrom
fix/mask-encoded-secrets

Conversation

@DmitryAnansky

@DmitryAnansky DmitryAnansky commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

What/Why/How?

Fixed secrets masking to cover encoded occurrences of a secret in har-output.

Reference

Testing

Screenshots (optional)

Check yourself

  • This PR follows the contributing guide
  • All new/updated code is covered by tests
  • Core code changed? - Tested with other Redocly products (internal contributions only)
  • New package installed? - Tested in different environments (browser/node)
  • Documentation update has been considered

Security

  • The security impact of the change has been considered
  • Code follows company security practices and guidelines

@changeset-bot

changeset-bot Bot commented Jul 31, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: ebc7074

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@redocly/respect-core Patch
@redocly/cli Patch
@redocly/openapi-core Patch
@redocly/client-generator Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@DmitryAnansky
DmitryAnansky force-pushed the fix/mask-encoded-secrets branch from a696d78 to fe080d1 Compare July 31, 2026 10:38
@DmitryAnansky DmitryAnansky self-assigned this Jul 31, 2026
@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Performance Benchmark (Lower is Faster)

CLI Version Bundle Lint Check Config
cli-latest ▓ 1.01x ± 0.02 ▓ 1.01x ± 0.01 ▓ 1.00x (Fastest)
cli-next ▓ 1.00x (Fastest) ▓ 1.00x (Fastest) ▓ 1.02x ± 0.02

@DmitryAnansky
DmitryAnansky marked this pull request as ready for review July 31, 2026 10:44
@DmitryAnansky
DmitryAnansky requested review from a team as code owners July 31, 2026 10:44
@DmitryAnansky
DmitryAnansky requested a review from Copilot July 31, 2026 10:49

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves secret masking in @redocly/respect-core logger output so that secrets are also masked when they appear in encoded forms (e.g., JSON-escaped or URL/form-urlencoded) within HAR captures.

Changes:

  • Add secret “variant” collection (raw, JSON-escaped, URL-encoded, form-urlencoded) and mask using those patterns.
  • Refactor string masking to apply consistently for both string targets and recursively-walked object values.
  • Add tests covering encoded secret occurrences and HAR-wide masking, plus a changeset for patch releases.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
packages/respect-core/src/modules/logger-output/mask-secrets.ts Adds encoded-variant pattern collection and updates masking to replace all occurrences across strings and nested objects.
packages/respect-core/src/modules/tests/logger-output/mask-secrets.test.ts Adds coverage for URL-encoded, JSON-escaped, repeated-occurrence, and HAR-capture masking scenarios.
.changeset/mask-encoded-secrets.md Publishes a patch changeset for the masking fix.

Comment thread packages/respect-core/src/modules/logger-output/mask-secrets.ts

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/respect-core/src/modules/logger-output/mask-secrets.ts:30

  • When multiple secrets (or their encoded variants) overlap (e.g., one is a substring of another), iterating patterns in insertion order can partially mask the longer secret first, leaving a suffix visible (e.g. replacing abc before abcd yields ********d). Sorting patterns by length (descending) before masking avoids this leakage and makes masking deterministic.
  return Array.from(patterns);

@github-actions

github-actions Bot commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 77.06% (🎯 77%) 11390 / 14779
🔵 Statements 77.18% (🎯 77%) 12176 / 15776
🔵 Functions 81.51% (🎯 81%) 2310 / 2834
🔵 Branches 70.76% (🎯 70%) 8331 / 11773
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/respect-core/src/modules/logger-output/mask-secrets.ts 93.22% 90.9% 100% 92.72% 98, 103-106
Generated in workflow #11051 for commit ebc7074 by the Vitest Coverage Report Action

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/respect-core/src/modules/logger-output/mask-secrets.ts:23

  • collectSecretPatterns skips only empty strings, but a whitespace-only secret (e.g. ' ') can still be added to secretsSet by other parts of respect-core. That would cause masking to replace spaces (and potentially + in the form-urlencoded variant) across the entire output, making logs unusable. Treat whitespace-only secrets as empty and skip them when building patterns.
  for (const secret of secretsSet) {
    if (!secret) continue;
    patterns.add(secret);

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (1)

packages/respect-core/src/modules/logger-output/mask-secrets.ts:29

  • collectSecretPatterns calls encodeURIComponent(secret) / URLSearchParams(...).toString() without guarding against URIError (thrown when the string contains malformed UTF-16 surrogate pairs). Since these secrets originate from captured traffic and headers, a single malformed value could crash masking/logging entirely. Consider wrapping the encoding steps in a try/catch and skipping the encoded variants on failure (the raw secret pattern will still be masked).
    patterns.add(secret);
    patterns.add(JSON.stringify(secret).slice(1, -1));
    patterns.add(encodeURIComponent(secret));
    // URLSearchParams serializes to `secret=value`; drop the key to keep
    // the form-urlencoded variant (spaces become `+`, unlike encodeURIComponent).
    patterns.add(new URLSearchParams([['secret', secret]]).toString().slice('secret='.length));
  }

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

Comment thread packages/respect-core/src/modules/logger-output/mask-secrets.ts
@adamaltman

Copy link
Copy Markdown
Member

What about the proxy command?

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.

4 participants