Skip to content

Redact secrets in decoded JSON values, not the serialized line - #682

Open
Inok wants to merge 1 commit into
mainfrom
pavel/ai-2262-json-safe-redaction
Open

Redact secrets in decoded JSON values, not the serialized line#682
Inok wants to merge 1 commit into
mainfrom
pavel/ai-2262-json-safe-redaction

Conversation

@Inok

@Inok Inok commented Aug 26, 2026

Copy link
Copy Markdown
Member

Closes #681 — AI-2262

What & why

SecretRedactor.RedactLine ran its patterns over the serialized JSONL line. Several value classes
exclude " and \ but not {, }, [, ], , or :, so a match beginning inside a string
value ran past it and swallowed the JSON that followed — and the server drops what it cannot parse
without reporting it, so transcripts lost turns invisibly. Redaction now walks tokens with a
Utf8JsonReader and re-emits every one through a Utf8JsonWriter, scanning only decoded string
values, which also keeps a serialized tool result carried as a string in scope. A secret-bearing key
arms its whole subtree rather than just a string directly under it, so {"auth":["b1","b2"]}
redacts both elements and keeps the array.

Where to look

Rewrite's token loop. The secretDepth arm/disarm is what keeps a secret key off its siblings —
a container reports the same depth on the way out as on the way in, which is what makes it exact.

One gap worth knowing, and one behaviour change:

shape behaviour
nested past 1000 (System.Text.Json's own ceiling) falls to the whole-line pipeline, with the corruption that implies — nothing in STJ can walk it
PEM key split across two string values ships intact, where the old pipeline destroyed it along with the surrounding JSON
Set-Cookie: session=\"x\" inside text still truncates at the quote — AI-649, which this does not close

Verification

The three shapes from #681, none of which parsed before:

{"headers":{"Cookie":{"session":"abc"},"Host":"example.com"},"tail":"kept"}
  → {"headers":{"Cookie":{"session":"[REDACTED]"},"Host":"example.com"},"tail":"kept"}
{"h":{"X-Api-Key":12345,"next":"kept"},"tail":"kept"}
  → {"h":{"X-Api-Key":-1,"next":"kept"},"tail":"kept"}

dotnet run --project test/Capacitor.Cli.Tests.Unit → 3738 passed, 0 failed, 19 skipped
dotnet run --project test/Capacitor.Cli.Core.Tests.Unit → 2372 passed, 0 failed, 2 skipped
dotnet publish src/Capacitor.Cli -c Release → no IL2xxx/IL3xxx

A seeded generative test runs 3000 documents through it: none throw, all outputs parse, and over a
third are actually rewritten. Same-process timing against the previous shape: 163 vs 165 µs on a
clean 13.8 KB line.

@linear-code

linear-code Bot commented Aug 26, 2026

Copy link
Copy Markdown

AI-2262

@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Redact secrets structurally within decoded JSON values

🐞 Bug fix 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Redacts secrets within decoded JSON values while preserving valid transcript structure.
• Propagates secret-key redaction through nested containers and protects failure paths.
• Adds structural, generative, and fidelity tests for parseability and untouched data.
Diagram

graph TD
  A["JSONL line"] --> B{"Oversize?"} -- "No" --> C["JSON reader"] -- "Valid" --> D["Structural redaction"] --> E["JSON writer"] --> F["Valid output"]
  B -- "Yes" --> H["Safe placeholder"]
  C -- "Invalid" --> G["Text fallback"] --> F
  E -- "Refused" --> H
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Parse into a JSON DOM
  • ➕ Simpler recursive subtree traversal
  • ➕ Straightforward key and value inspection
  • ➖ Adds full-document allocations
  • ➖ Risks normalizing untouched numbers and formatting
  • ➖ Requires deliberate handling of duplicate property names
2. Patch original UTF-8 byte ranges
  • ➕ Could preserve all untouched serialization exactly
  • ➕ Avoids re-encoding the full changed document
  • ➖ Decoded matches must be mapped back through JSON escapes
  • ➖ Structural subtree replacement becomes substantially more complex
  • ➖ Higher risk of producing malformed JSON

Recommendation: Keep the streaming Utf8JsonReader/Utf8JsonWriter approach. It provides the strongest parseability guarantee with bounded memory, preserves unchanged lines byte-for-byte, and explicitly retains raw numeric spelling when a rewrite occurs; the DOM and byte-patching alternatives trade away fidelity, efficiency, or safety.

Files changed (6) +596 / -72

Bug fix (1) +278 / -72
SecretRedactor.csRewrite JSON redaction as a token-preserving pipeline +278/-72

Rewrite JSON redaction as a token-preserving pipeline

• Replaces serialized-line scanning with Utf8JsonReader and Utf8JsonWriter token mirroring, redacting decoded string values and all string or numeric leaves under secret-bearing keys. It also redacts credential property names distinctly, preserves untouched numeric spelling and unchanged lines, expands shared key vocabularies, and adds safe fallback or placeholder behavior for unreadable and unwritable data.

src/Capacitor.Cli/SecretRedactor.cs

Tests (1) +271 / -0
SecretRedactorTests.csCover structural redaction, parseability, and fidelity +271/-0

Cover structural redaction, parseability, and fidelity

• Adds nested subtree, sibling isolation, key spelling, credential property-name, escaped inner JSON, duplicate-key, deep-input, and number-format tests. A seeded 3,000-document generative test verifies the redactor never throws, always emits parseable JSON, and actually performs redactions.

test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs

Documentation (2) +45 / -0
CLAUDE.mdDocument the structural redaction safety invariant +4/-0

Document the structural redaction safety invariant

• Adds a deliberate design constraint requiring redaction of decoded JSON string values rather than serialized lines. It also records the fail-closed placeholder behavior when rewritten output is rejected.

CLAUDE.md

CHANGES.mdExplain structural redaction behavior and tradeoffs +41/-0

Explain structural redaction behavior and tradeoffs

• Documents the corruption root cause, token-based rewrite design, subtree semantics, fidelity guarantees, fallback behavior, and known limitations.

docs/CHANGES.md

Other (2) +2 / -0
Directory.Packages.propsCentrally pin the DotNext package version +1/-0

Centrally pin the DotNext package version

• Adds DotNext 6.6.2 to central package management for pooled array buffer writers used by structural redaction.

Directory.Packages.props

Capacitor.Cli.csprojReference DotNext for pooled JSON buffers +1/-0

Reference DotNext for pooled JSON buffers

• Adds the DotNext package dependency required by the redactor's pooled UTF-8 input and output buffers.

src/Capacitor.Cli/Capacitor.Cli.csproj

@qodo-code-review

qodo-code-review Bot commented Aug 26, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (2) 📜 Skill insights (0)

Grey Divider


Action required

1. Comments produce invalid JSON 📘 Rule violation ≡ Correctness ⭐ New
Description
The rewrite re-emits comments with WriteCommentValue, so a line containing a comment and any
redacted value is returned as JSON-with-comments that the server's strict parser can drop. This
violates the requirement that redaction preserve valid JSON output.
Code

src/Capacitor.Cli/SecretRedactor.cs[R158-161]

+                    // Dropping it still counts as a change, or the unchanged path hands back the
+                    // raw line with this comment, and whatever is in it, still there.
+                    if (text.Contains("*/", StringComparison.Ordinal)) redactedAny = true;
+                    else writer.WriteCommentValue(text);
Evidence
Rule 13 requires redacted output to preserve valid JSON. The reader explicitly surfaces comments,
and the changed comment branch writes them back with WriteCommentValue; when redactedAny is
true, that JSON-with-comments buffer is returned even though strict parsers reject it.

CLAUDE.md: Redact Secrets Within Decoded JSON Values
src/Capacitor.Cli/SecretRedactor.cs[151-161]
src/Capacitor.Cli/SecretRedactor.cs[185-186]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The structural rewrite re-emits comments using `Utf8JsonWriter.WriteCommentValue`, producing JSON-with-comments rather than strict JSON whenever another token causes the rewritten buffer to be returned.

## Issue Context
`JsonDocument.Parse` and the server's strict JSON parser reject comments by default. Comments may still be scanned for secrets, but a rewritten line must omit them or otherwise emit strict, parseable JSON.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[148-163]
- src/Capacitor.Cli/SecretRedactor.cs[182-189]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[960-975]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Escaped secret tails leak ✗ Dismissed 🐞 Bug ⛨ Security ⭐ New
Description
JsonKeySecretRegex treats \" inside a decoded serialized-JSON value as the value's closing
delimiter, so a secret such as {"client_secret":"abc\"sensitive-tail"} is only redacted through
abc and uploads sensitive-tail. A backslash not followed by a quote can prevent this key-based
matcher from redacting the value at all.
Code

src/Capacitor.Cli/SecretRedactor.cs[305]

+        """((?:\\"|")(?:[^"\\]*(?:""" + SecretKeywords + """)[^"\\]*)(?:\\"|")[ \t]*:[ \t]*(?:\\"|"))([^"\\]+)((?:\\"|")|$)""",
Evidence
The implementation explicitly scans decoded outer string values and uses this regex for both
prefiltering and replacement. The regex's value body excludes backslashes while its closing group
accepts \", so the first escaped quote is indistinguishable from the actual embedded-JSON closing
quote; the existing escaped-inner-JSON test only uses a value without internal escapes and therefore
does not cover the leaking shape.

src/Capacitor.Cli/SecretRedactor.cs[102-109]
src/Capacitor.Cli/SecretRedactor.cs[214-225]
src/Capacitor.Cli/SecretRedactor.cs[227-238]
src/Capacitor.Cli/SecretRedactor.cs[299-310]
test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[838-856]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`JsonKeySecretRegex` cannot safely identify the end of a quoted value containing JSON escape sequences. It may treat an escaped quote as the closing delimiter or fail at a backslash, leaving secret content exposed.

## Issue Context
The outer JSON reader decodes string values before this regex runs, while serialized JSON carried inside such a value still contains its own escape sequences. Key-classified values in that embedded JSON must be redacted in full.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[299-310]
- src/Capacitor.Cli/SecretRedactor.cs[214-238]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[838-870]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Numeric secrets remain exposed ✗ Dismissed 🐞 Bug ⛨ Security
Description
The Number branch writes every numeric value unchanged even when its property is classified as
secret-bearing, so inputs such as {"X-Api-Key":12345} now upload the credential verbatim. The
blanket exemption added for token-usage metrics therefore creates a security bypass for numeric API
keys, passwords, and tokens.
Code

src/Capacitor.Cli/SecretRedactor.cs[117]

+                case JsonTokenType.Number: writer.WriteRawValue(reader.ValueSpan, skipInputValidation: true); break;
Evidence
Secret-bearing properties are identified by IsSecretBearingKey, but unlike strings, the number
token path never consults keyIsSecret or inSecret and writes the original token directly.
RedactLine is applied to transcript lines before forwarding, so such numeric credentials leave the
host unchanged.

src/Capacitor.Cli/SecretRedactor.cs[87-90]
src/Capacitor.Cli/SecretRedactor.cs[102-117]
src/Capacitor.Cli/SecretRedactor.cs[202-203]
src/Capacitor.Cli/WatcherManager.cs[643-661]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Numeric values under secret-bearing keys are emitted unchanged, allowing numeric credentials to reach the server.

## Issue Context
The exemption was intended to preserve token-usage metrics, but it currently applies to every numeric secret-key value and secret subtree. Preserve known metric fields explicitly rather than treating all JSON numbers as non-sensitive.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[113-117]
- src/Capacitor.Cli/SecretRedactor.cs[202-203]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[791-800]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View high (3)
4. Token metrics are redacted ✗ Dismissed 🐞 Bug ≡ Correctness
Description
SecretKeyNameRegex matches secret keywords anywhere in a property name, so normal Codex fields
such as token_count, total_token_usage, and input_tokens are classified as secret-bearing and
their numeric values or entire subtrees are replaced with -1. Because every drained transcript
line passes through RedactLine, this silently corrupts real token-usage events sent to the server.
Code

src/Capacitor.Cli/SecretRedactor.cs[276]

+    [GeneratedRegex("(?:" + SecretKeywords + ")", RegexOptions.IgnoreCase)]
Evidence
The regex has no anchors or component boundaries, while the repository's real Codex fixture contains
several property names with the token substring. IsSecretBearingKey applies that regex to every
property, and the live drain maps every outgoing line through RedactLine, proving these metric
values are rewritten in production.

src/Capacitor.Cli/SecretRedactor.cs[209-212]
src/Capacitor.Cli/SecretRedactor.cs[254-261]
src/Capacitor.Cli/SecretRedactor.cs[276-279]
test/Capacitor.Cli.Tests.Unit/Harness/Codex/CodexSubagentTurnTrackerTests.cs[19-27]
src/Capacitor.Cli/Commands/WatchCommand.cs[1979-1981]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The structural property-name matcher is unanchored, causing ordinary usage fields containing `token` to be treated as credential keys and rewritten.

## Issue Context
Real Codex rollout lines contain `token_count`, `total_token_usage`, and `input_tokens`. Property matching must retain intended variants such as `privateKey` while excluding unrelated compound names used for metrics.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[254-279]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[760-789]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


5. JsonException restores raw-line redaction 📘 Rule violation ⛨ Security
Description
When structural parsing fails, RedactLine sends the serialized JSON line through RedactSecrets
instead of returning the safe placeholder. This can expose rejected input or reproduce the
malformed-output corruption that decoded-value redaction is required to prevent.
Code

src/Capacitor.Cli/SecretRedactor.cs[R37-40]

+        } catch (JsonException) {
+            // Not JSON, or nested past the reader's limit. Either way there is no structure left
+            // to work with, and the whole-line pipeline is all that remains.
+            return RedactSecrets(rawJsonlLine);
Evidence
PR Compliance ID 12 requires decoded-value redaction and a safe placeholder for refused lines, while
the changed JsonException path explicitly invokes the whole-line RedactSecrets(rawJsonlLine)
pipeline.

CLAUDE.md: Redact Secrets Within Decoded JSON Values and Never Fall Back to Raw Data
src/Capacitor.Cli/SecretRedactor.cs[37-40]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedactLine` catches `JsonException` and falls back to redacting the serialized input, violating the requirement to redact only decoded JSON values and never return rejected raw data.

## Issue Context
Malformed JSON and documents beyond the configured reader depth cannot be structurally redacted safely. Return a non-sensitive placeholder for these cases, and update the deep-nesting test to assert the placeholder rather than text-pipeline redaction.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[37-40]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[923-934]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


6. Comment secrets bypass redaction ✗ Dismissed 🐞 Bug ⛨ Security
Description
CommentHandling.Skip silently removes comments from the token stream, but when no value changes
Rewrite returns false and RedactLine returns the original line, so a line such as `{"a":1/*
ghp_... */}` uploads the secret unchanged. Previously the whole-line regex pipeline scanned that
comment text.
Code

src/Capacitor.Cli/SecretRedactor.cs[175]

+        new() { MaxDepth = 128, CommentHandling = JsonCommentHandling.Skip };
Evidence
The reader is explicitly configured to skip comments, while Rewrite only scans property names and
string values and reports a change solely through redactedAny. A false result causes RedactLine
to return the original raw line, and the caller forwards that result in transcript batches, so
comment contents are transmitted without ever reaching RedactSecrets.

src/Capacitor.Cli/SecretRedactor.cs[35-40]
src/Capacitor.Cli/SecretRedactor.cs[64-66]
src/Capacitor.Cli/SecretRedactor.cs[74-109]
src/Capacitor.Cli/SecretRedactor.cs[166-175]
src/Capacitor.Cli/Commands/WatchCommand.cs[1979-1981]
src/Capacitor.Cli/WatcherManager.cs[673-685]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Secrets that occur only in JSON comments bypass redaction because the reader skips those comments and the unchanged fast path returns the original line.

## Issue Context
Commented input should not be accepted structurally unless comment text is also scanned. The simplest safe behavior is to disallow comments so `JsonException` routes the complete line through the existing text-redaction fallback; alternatively, explicitly process and safely rewrite comments.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[35-40]
- src/Capacitor.Cli/SecretRedactor.cs[64-66]
- src/Capacitor.Cli/SecretRedactor.cs[171-175]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[665-935]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context sources
Review mode: ⚖️ Balanced: The latest push substantially rewrites secret-redaction parsing across multiple token, fallback, regex, and failure paths, creating real security and data-integrity risk; it warrants a careful single-pass review, but not redundant extended passes.

Grey Divider

Tip of the day
💡 Did you know, you can ask Qodo to dismiss a finding you disagree with, with your reason on record

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Previous reviews

Review updated until commit 6b89a48

Results up to commit 45fd9a3 ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (1) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. JsonException restores raw-line redaction 📘 Rule violation ⛨ Security
Description
When structural parsing fails, RedactLine sends the serialized JSON line through RedactSecrets
instead of returning the safe placeholder. This can expose rejected input or reproduce the
malformed-output corruption that decoded-value redaction is required to prevent.
Code

src/Capacitor.Cli/SecretRedactor.cs[R37-40]

+        } catch (JsonException) {
+            // Not JSON, or nested past the reader's limit. Either way there is no structure left
+            // to work with, and the whole-line pipeline is all that remains.
+            return RedactSecrets(rawJsonlLine);
Evidence
PR Compliance ID 12 requires decoded-value redaction and a safe placeholder for refused lines, while
the changed JsonException path explicitly invokes the whole-line RedactSecrets(rawJsonlLine)
pipeline.

CLAUDE.md: Redact Secrets Within Decoded JSON Values and Never Fall Back to Raw Data
src/Capacitor.Cli/SecretRedactor.cs[37-40]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`RedactLine` catches `JsonException` and falls back to redacting the serialized input, violating the requirement to redact only decoded JSON values and never return rejected raw data.

## Issue Context
Malformed JSON and documents beyond the configured reader depth cannot be structurally redacted safely. Return a non-sensitive placeholder for these cases, and update the deep-nesting test to assert the placeholder rather than text-pipeline redaction.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[37-40]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[923-934]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Comment secrets bypass redaction ✗ Dismissed 🐞 Bug ⛨ Security
Description
CommentHandling.Skip silently removes comments from the token stream, but when no value changes
Rewrite returns false and RedactLine returns the original line, so a line such as `{"a":1/*
ghp_... */}` uploads the secret unchanged. Previously the whole-line regex pipeline scanned that
comment text.
Code

src/Capacitor.Cli/SecretRedactor.cs[175]

+        new() { MaxDepth = 128, CommentHandling = JsonCommentHandling.Skip };
Evidence
The reader is explicitly configured to skip comments, while Rewrite only scans property names and
string values and reports a change solely through redactedAny. A false result causes RedactLine
to return the original raw line, and the caller forwards that result in transcript batches, so
comment contents are transmitted without ever reaching RedactSecrets.

src/Capacitor.Cli/SecretRedactor.cs[35-40]
src/Capacitor.Cli/SecretRedactor.cs[64-66]
src/Capacitor.Cli/SecretRedactor.cs[74-109]
src/Capacitor.Cli/SecretRedactor.cs[166-175]
src/Capacitor.Cli/Commands/WatchCommand.cs[1979-1981]
src/Capacitor.Cli/WatcherManager.cs[673-685]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Secrets that occur only in JSON comments bypass redaction because the reader skips those comments and the unchanged fast path returns the original line.

## Issue Context
Commented input should not be accepted structurally unless comment text is also scanned. The simplest safe behavior is to disallow comments so `JsonException` routes the complete line through the existing text-redaction fallback; alternatively, explicitly process and safely rewrite comments.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[35-40]
- src/Capacitor.Cli/SecretRedactor.cs[64-66]
- src/Capacitor.Cli/SecretRedactor.cs[171-175]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[665-935]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit 18ef70f ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Token metrics are redacted ✗ Dismissed 🐞 Bug ≡ Correctness
Description
SecretKeyNameRegex matches secret keywords anywhere in a property name, so normal Codex fields
such as token_count, total_token_usage, and input_tokens are classified as secret-bearing and
their numeric values or entire subtrees are replaced with -1. Because every drained transcript
line passes through RedactLine, this silently corrupts real token-usage events sent to the server.
Code

src/Capacitor.Cli/SecretRedactor.cs[276]

+    [GeneratedRegex("(?:" + SecretKeywords + ")", RegexOptions.IgnoreCase)]
Evidence
The regex has no anchors or component boundaries, while the repository's real Codex fixture contains
several property names with the token substring. IsSecretBearingKey applies that regex to every
property, and the live drain maps every outgoing line through RedactLine, proving these metric
values are rewritten in production.

src/Capacitor.Cli/SecretRedactor.cs[209-212]
src/Capacitor.Cli/SecretRedactor.cs[254-261]
src/Capacitor.Cli/SecretRedactor.cs[276-279]
test/Capacitor.Cli.Tests.Unit/Harness/Codex/CodexSubagentTurnTrackerTests.cs[19-27]
src/Capacitor.Cli/Commands/WatchCommand.cs[1979-1981]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The structural property-name matcher is unanchored, causing ordinary usage fields containing `token` to be treated as credential keys and rewritten.

## Issue Context
Real Codex rollout lines contain `token_count`, `total_token_usage`, and `input_tokens`. Property matching must retain intended variants such as `privateKey` while excluding unrelated compound names used for metrics.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[254-279]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[760-789]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Results up to commit d7f2cfa ⚖️ Balanced


🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)


Action required
1. Numeric secrets remain exposed ✗ Dismissed 🐞 Bug ⛨ Security
Description
The Number branch writes every numeric value unchanged even when its property is classified as
secret-bearing, so inputs such as {"X-Api-Key":12345} now upload the credential verbatim. The
blanket exemption added for token-usage metrics therefore creates a security bypass for numeric API
keys, passwords, and tokens.
Code

src/Capacitor.Cli/SecretRedactor.cs[117]

+                case JsonTokenType.Number: writer.WriteRawValue(reader.ValueSpan, skipInputValidation: true); break;
Evidence
Secret-bearing properties are identified by IsSecretBearingKey, but unlike strings, the number
token path never consults keyIsSecret or inSecret and writes the original token directly.
RedactLine is applied to transcript lines before forwarding, so such numeric credentials leave the
host unchanged.

src/Capacitor.Cli/SecretRedactor.cs[87-90]
src/Capacitor.Cli/SecretRedactor.cs[102-117]
src/Capacitor.Cli/SecretRedactor.cs[202-203]
src/Capacitor.Cli/WatcherManager.cs[643-661]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Numeric values under secret-bearing keys are emitted unchanged, allowing numeric credentials to reach the server.

## Issue Context
The exemption was intended to preserve token-usage metrics, but it currently applies to every numeric secret-key value and secret subtree. Preserve known metric fields explicitly rather than treating all JSON numbers as non-sensitive.

## Fix Focus Areas
- src/Capacitor.Cli/SecretRedactor.cs[113-117]
- src/Capacitor.Cli/SecretRedactor.cs[202-203]
- test/Capacitor.Cli.Tests.Unit/SecretRedactorTests.cs[791-800]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Qodo Logo

Comment thread src/Capacitor.Cli/SecretRedactor.cs
Comment thread src/Capacitor.Cli/SecretRedactor.cs Outdated
@Inok
Inok force-pushed the pavel/ai-2262-json-safe-redaction branch 4 times, most recently from 0c80353 to 18ef70f Compare August 26, 2026 23:25
@Inok

Inok commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread src/Capacitor.Cli/SecretRedactor.cs Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 18ef70f

@Inok
Inok force-pushed the pavel/ai-2262-json-safe-redaction branch from 18ef70f to d7f2cfa Compare August 26, 2026 23:48
@Inok

Inok commented Aug 26, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread src/Capacitor.Cli/SecretRedactor.cs Outdated
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit d7f2cfa

@Inok
Inok force-pushed the pavel/ai-2262-json-safe-redaction branch 2 times, most recently from 315b7e9 to f544928 Compare August 27, 2026 00:13
@Inok

Inok commented Aug 27, 2026

Copy link
Copy Markdown
Member Author

/agentic_review

Comment thread src/Capacitor.Cli/SecretRedactor.cs Outdated
Comment thread src/Capacitor.Cli/SecretRedactor.cs
@qodo-code-review

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit f544928

A pattern run over the serialized line matches past the value it found into
the surrounding structure, and the server drops an unparseable line without a
word. Mirroring every token through a Utf8JsonWriter makes malformed output
unrepresentable. The reader's depth limit is the writer's own ceiling, so what
one accepts the other can emit and the fallback sees only unparseable input.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@Inok
Inok force-pushed the pavel/ai-2262-json-safe-redaction branch from f544928 to 6b89a48 Compare August 27, 2026 09:57
@Inok
Inok requested a review from alexeyzimarev August 27, 2026 10:10
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.

secret redaction runs regexes over the serialized JSON line and can emit invalid JSON the server silently drops

1 participant