You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maka.apache.org by "Totoro-qaq (via GitHub)" <gi...@apache.org> on 2026/09/17 05:26:57 UTC
[PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Totoro-qaq opened a new pull request, #5434:
URL: https://github.com/apache/maka/pull/5434
## Summary
`redactSecrets` applied its `key=value` assignment pattern with one global replace. A harmless key such as `excerpt` matched together with its whole value. So in `Config excerpt: password=...`, the nested `password=` had already been consumed and was never tested.
- Assignment prefixes (key, separator, optional quote) are now scanned in a loop. A harmless key's value is still searched for nested sensitive assignments, and a sensitive key's value is redacted once. A value that starts inside an already redacted value is not redacted again.
- The scan stays linear on long key-character runs even though values are now searched:
- Each `[A-Za-z0-9_-]` run is matched once, and the key is taken from its first word-initial letter. That is the same key the old `\b` start produced.
- `sensitiveKeySegments` splits uppercase runs with a linear regex.
- A 256 KB base64url blob that took about 3 s on main now takes about 5 ms.
- The other patterns, their order, the sensitive-key rules and the value terminators are unchanged.
Behavior change: an assignment nested in another value is now redacted the way it already was on its own. `url=https://h/p;password=x` redacts the password, and `shortcut=key:Enter` becomes `shortcut=key:[redacted]`.
Note for #4930: its new `QUOTED_SECRET_ASSIGNMENT_PATTERN` starts keys with the old `\b[A-Za-z]...` form, which is quadratic on hyphenated runs. The bounded-time test here will flag that if both land. The key-run prefix used here avoids it.
Fixes #5433
## Verification
- New tests in `redaction.test.ts`:
- 8 nested forms, each unchanged on main;
- harmless values that must stay unchanged;
- sensitive keys without a value;
- an empty value followed by another assignment (`env: API_TOKEN= DB_PASSWORD="..."`), which must still redact the password;
- redacting twice gives the same result;
- a bounded-time test on 200k-character hyphenated, uppercase and base64 inputs.
- Mutation checks on the new guards. Two mutants survive:
- the unreachable `valueStart <= copied` boundary;
- a key-start variant that differs only for keys beginning with `_` or a digit.
- Differential fuzz against main:
- 600k inputs: every difference is added redaction of a nested assignment.
- 800k leak-fuzz inputs: 0 secrets shown. An earlier version of this change leaked 1,074 of 400k; that is fixed.
- 1.5M short inputs: redacting the output again changes nothing (120 such cases on main).
- Package tests:
- `@maka/core` 851/851, `@maka/mcp` 250/250, `@maka/runtime` 0 failures (3,521 passed, 14 skipped), `@maka/ui` 491/491.
- `@maka/storage` has one failure in `managed-dependency-environment-crash` (a Node 22 SQLite warning on child stderr).
- `@maka/runtime-host` has 6 cancelled tests in `resumable-peer-stream`. The runtime-host result is the same without this change.
- Checked together with the Deep Research fix for #5432: the two commits cherry-pick cleanly, and core passes 852/852.
- Core typecheck, biome lint and format, `git diff --check` and the Windows test inventory pass.
## AI use
- [ ] No generative tool made a substantive contribution
- [x] Generative tooling made a substantive contribution
Tool(s) and scope: Claude Code helped investigate, implement and test this change. The commit carries a `Generated-by: Claude Code` trailer.
## Checklist
- [x] Tests cover the change and fail without it
- [x] Lint, format, typecheck and the affected suites pass locally
Does this PR entail a change in behavior?
- [x] Yes — described under Summary above
- [ ] No
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Posted by "Totoro-qaq (via GitHub)" <gi...@apache.org>.
Totoro-qaq commented on PR #5434:
URL: https://github.com/apache/maka/pull/5434#issuecomment-5712471862
Thanks for the thorough pass. Addressed in 321c2a8c:
1. `redactAssignedSecrets` now takes the sticky value match with `exec` and skips the assignment if it ever fails. `copied` only advances past a value it actually matched, so a failed sticky match can no longer reset the copy point and echo the value.
2. The bounded-time test now includes a bare hyphenated run. I measured before choosing the input:
- On main with n = 30,000, `'a-'.repeat(n)` takes about 1.9 s, both bare and after `x `.
- `note: …` and `data=…` take about 1 ms on main, because the outer assignment still swallows the run.
- With this change, all four take about 1 ms.
So I added `'a-'.repeat(100_000)` rather than the `note:` form. Pointed at main's redaction, the test now fails its bound (`scanned in 21082ms`). I kept the elapsed assertion rather than `{ timeout }`, because `node:test` cannot interrupt synchronous work, so a timeout would not stop a slow regex either.
3. Agreed on the `API_TOKEN= DB_PASSWORD="…"` output. Leaving it as is.
On `_password=` / `_token=`: agreed that it predates this PR. I'll keep it out of scope here.
_Claude Code-assisted._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han commented on PR #5434:
URL: https://github.com/apache/maka/pull/5434#issuecomment-5862368890
Thanks @Totoro-qaq — the automated review of head `82cfe2d4` found no blocking issues, CI is green, and it merges cleanly. Approving and squash-merging at @Astro-Han's request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Posted by "Astro-Han (via GitHub)" <gi...@apache.org>.
Astro-Han merged PR #5434:
URL: https://github.com/apache/maka/pull/5434
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Posted by "Totoro-qaq (via GitHub)" <gi...@apache.org>.
Totoro-qaq commented on PR #5434:
URL: https://github.com/apache/maka/pull/5434#issuecomment-5861422655
Addressed both P3 notes in 82cfe2d4:
- The display/backend comment now describes the backend's key-aware rules and its intentional masking of nested sensitive-key lookalikes.
- The long-input regression now measures process CPU time instead of wall-clock time, so runner descheduling does not consume its budget. The 5-second guard remains unchanged.
The guard still distinguishes the old implementation: the same four inputs used about 30ms CPU with the fix and 23.7s on the PR base on this machine. The fresh Core suite passes 851/851. The root build, typecheck, lint, format check, Desktop/UI knip checks, and commit hooks also pass. These are local results; hosted CI for the new commit has not yet completed.
Codex-assisted follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org
Re: [PR] fix(core): redact a secret assignment nested in a harmless assignment's value [maka]
Posted by "Totoro-qaq (via GitHub)" <gi...@apache.org>.
Totoro-qaq commented on PR #5434:
URL: https://github.com/apache/maka/pull/5434#issuecomment-5862583734
Updated the PR's AI use section to retain Claude Code's attribution for the initial fix and explicitly credit Codex for follow-up `82cfe2d4`.
That follow-up covers two P3 items:
- Clarified the display/backend redaction comment, including the intentional handling of nested sensitive-key lookalikes.
- Changed the long-input regression from wall-clock timing to process CPU time, retaining the same 5-second budget.
The follow-up changes documentation and test timing; the redaction behavior is unchanged. Validation results are recorded in the [earlier follow-up comment](https://github.com/apache/maka/pull/5434#issuecomment-5861422655).
Codex-assisted comment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org