Address CodeRabbit review: redact-before-truncate, unmount feedback, label truncation flag

- Redact oversized extra records before truncation so a sliced secret
  fragment can never evade the pattern redactor on the upload path
- Report perf-dump success (toast + reveal) even if Settings unmounted
  during the ~10s capture; the artifact is already on disk
- Only flag worktree labels as truncated when the 80-char cap actually
  dropped data, not on whitespace trimming
- Document the fixed 64 KiB extra-record budget; fix a 9-char locale key

Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
brennanb2025
2026-07-03 17:57:36 -07:00
co-authored by Orca
parent 9fadc8deeb
commit 54f6785237
5 changed files with 44 additions and 4 deletions
+31
View File
@@ -255,6 +255,37 @@ describe('bundle — collection', () => {
expect(bundle.payload).toContain('"name":"kept"')
})
it('redacts secrets in oversized extra records before truncation can bisect them', () => {
writeFileSync(traceFile, makeNDJSON([makeSpan({ name: 'kept' })]))
// A secret placed past the 1024-char string-truncation boundary: if
// truncation ran before redaction, the sliced fragment would no longer
// pattern-match and partial key material could ride the upload.
const secret = `sk-ant-api03-${'a'.repeat(80)}`
const bundle = collectBundle({
traceFilePath: traceFile,
maxFiles: 10,
appVersion: '1',
platform: 'darwin',
arch: 'arm64',
osRelease: '24',
orcaChannel: 'dev',
extraRecords: [
{
type: 'renderer-perf',
note: `${'x'.repeat(1000)}${secret}`,
payload: 'y'.repeat(_internalsForTests.MAX_EXTRA_RECORD_BYTES * 2)
}
]
})
expect(bundle.payload).not.toContain('sk-ant-api03-')
const extraLine = bundle.payload
.split('\n')
.filter(Boolean)
.map((line) => JSON.parse(line))[1]
expect(extraLine.type).toBe('renderer-perf')
expect(extraLine.truncated).toBe(true)
})
it('does not append a span that would push the payload over the upload cap', () => {
const giantSpan = makeSpan({
attributes: {
+5 -1
View File
@@ -71,6 +71,8 @@ type BundleHeader = {
readonly schema_version: 1
}
// Why: extra records are metrics riders, not span payload — a fixed 64 KiB
// ceiling keeps them from crowding spans out of the 4 MiB bundle.
const MAX_EXTRA_RECORD_BYTES = 64 * 1024
function* readLinesNewestFirst(text: string): Iterable<string> {
@@ -250,7 +252,9 @@ function serializeExtraRecord(
return serialized
}
const truncated = JSON.stringify(redactValue(markExtraRecordTruncated(record), 'server'))
// Why: truncate the already-redacted value — truncating first could bisect
// a long secret so the redactor no longer pattern-matches the fragment.
const truncated = JSON.stringify(markExtraRecordTruncated(redacted))
if (Buffer.byteLength(truncated) + 1 <= budget) {
return truncated
}
@@ -252,9 +252,11 @@ export function PrivacyDiagnosticsSection(): React.JSX.Element {
setPerfDumpStage(null)
try {
const result = await window.api.diagnostics.capturePerfDump()
if (!mountedRef.current || 'canceled' in result) {
if ('canceled' in result) {
return
}
// Why: capture runs ~10+ s and the artifact is already on disk —
// report it (global toast + reveal) even if Settings unmounted.
toast.success(
translate(
'auto.components.settings.PrivacyDiagnosticsSection.c4cd74809e',
@@ -63,7 +63,7 @@ export const getPrivacyPaneSearchEntries = createLocalizedCatalog(() => [
...translateSearchKeyword('auto.components.settings.privacy.search.048b49260d', 'heap'),
...translateSearchKeyword('auto.components.settings.privacy.search.c1d94c8566', 'trace'),
...translateSearchKeyword('auto.components.settings.privacy.search.ec083fb9e1', 'slow'),
...translateSearchKeyword('auto.components.settings.privacy.search.07241d4d1', 'lag')
...translateSearchKeyword('auto.components.settings.privacy.search.900d529b12', 'lag')
]
},
{
+4 -1
View File
@@ -191,7 +191,10 @@ function narrowWorktree(value: unknown): [RendererPerfWorktreeMetrics, boolean]
scrollbackRowsTotal: clampRendererPerfCount(value.scrollbackRowsTotal),
panes
},
rawPanes.length > panes.length || label.length < String(value.worktreeLabel ?? '').length
// Compare against the trimmed input so whitespace padding does not
// count as truncation — `truncated` must mean data was dropped.
rawPanes.length > panes.length ||
(typeof value.worktreeLabel === 'string' && label.length < value.worktreeLabel.trim().length)
]
}