fix: redact secrets embedded in JSON log strings

[cursor]

Description

Fixes a debug-log secret exposure path where successfully parsed JSON request/response bodies only redacted sensitive object keys. JSON fields such as pipeline_yaml could contain YAML block scalar secrets (privateKey: |) and be logged verbatim at debug level.

The fix scrubs primitive string values during recursive redaction and uses line-based YAML block scalar redaction so safe same-indent YAML fields remain visible.

Type of Change

• Bug fix
• New feature
• Refactor
• Documentation
• Other

Checklist

• Tests pass
• Typecheck passes

Validation:

• pnpm test tests/utils/redact.test.ts
• pnpm typecheck
• pnpm test
• pnpm build

  

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[cursor]

Findings:

• Critical — src/utils/redact.ts: the new string-scrubbing path still leaks non-scalar secrets embedded inside string fields. I reproduced redactJsonString(JSON.stringify({ pipeline_yaml: 'secret: value: abc name: safe' })), which returns the secret unchanged, and secret: {"value":"abc"}, which becomes secret: [REDACTED]{"value":"abc"} and still exposes the secret payload. That violates the repo rule to never expose secret values and shows the fallback/string path is still diverging from the exact-key structured redaction path.

Architecture fit:

• The change is nicely scoped and preserves layering by keeping the behavior inside src/utils/redact.ts with focused tests; it also does not touch the global instruction surface.
• I do not think it fully meets the architecture/security bar yet, though, because the remaining regex-driven string fallback is still ad hoc and incomplete for structured values.

Missing tests / verification gaps:

• Please add regression coverage for string payloads where a sensitive key owns a nested mapping, inline object, or array, for example secret: value: abc, secret: {"value":"abc"}, and secret: ["abc"]. The current new tests cover block scalars, but not these structured forms, which is why the remaining leak slips through.

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

Critical: this helper still leaks structured secret values inside ordinary string fields.

Concrete repros I ran against the branch:

redactJsonString(JSON.stringify({
  pipeline_yaml: "secret:
  value: abc
name: safe",
}));
// => {"pipeline_yaml":"secret:
  value: abc
name: safe"}

redactJsonString('secret: {"value":"abc"}');
// => 'secret: [REDACTED]{"value":"abc"}'

Because scrubSensitiveText() only combines block-scalar handling with token regex replacement, the string fallback is still not aligned with redactSensitiveFields()' whole-value redaction for sensitive keys. That means this PR still leaves a real secret-leak path in debug logs for mapping/object/array-style values.

[cursor]

Findings:

• Critical — src/utils/redact.ts: the new string-scrubbing path still leaks structured secrets embedded inside string fields, so this change does not yet meet the repo rule to never expose secret values. I reproduced all of the following on the current PR code:
  • redactJsonString(JSON.stringify({ pipeline_yaml: 'secret: value: abc name: safe' })) returns the secret unchanged.
  • secret: {"value":"abc"} becomes secret: [REDACTED]{"value":"abc"} and still exposes the payload.
  • secret: ["abc"] becomes secret: [REDACTED]] and still leaks structure/content.
    The new test and green CI only cover block scalars, so these cases still slip through.

Assumptions / open questions:

• I am assuming the intended contract is conservative redaction: once a sensitive key is detected inside a free-form string, the entire associated value needs to be scrubbed even when that value is a nested mapping, inline object, or array. If that assumption is right, the current regex-only fallback is too narrow.

Change summary:

• The PR is otherwise well scoped and keeps the work isolated to src/utils/redact.ts with focused coverage in tests/utils/redact.test.ts.
• Fresh verification I ran locally on this branch: pnpm test tests/utils/redact.test.ts (24/24 passing) and pnpm typecheck (passing).

  

_{Sent by Cursor Automation: Sunil On Demand Architecture Review}

[cursor]

This helper is still only safe for block scalars plus flat scalar key/value pairs. It misses structured values after a sensitive key, which means secrets still leak on the current branch. Repros:

redactJsonString(JSON.stringify({ pipeline_yaml: 'secret:
  value: abc
name: safe' }))
// => {"pipeline_yaml":"secret:
  value: abc
name: safe"}

redactJsonString(JSON.stringify({ pipeline_yaml: 'secret: {"value":"abc"}
name: safe' }))
// => {"pipeline_yaml":"secret: [REDACTED]{\"value\":\"abc\"}
name: safe"}

That still falls short of the repo's never expose secret values standard. We need to treat a sensitive key followed by a structured value as fully redacted, not rely on a regex that only models scalars.

[cursor]

This is a good regression for block scalars, but it still leaves the structured-string cases untested. Please add cases for a sensitive key followed by a nested mapping, inline object, and array, e.g. secret: value: abc, secret: {"value":"abc"}, and secret: ["abc"]. The current implementation leaks those on this branch even though this test passes.