fix(observability): redact free-form secrets - #520
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ea5309a796
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
✅ Watchdog review — approve
Reviewed commit ea5309a7967dfa897150d3616b932e5bd2319c11 for correctness, security, regressions, and tests.
결과
- Structured and free-form observability/doctor output is redacted, including GitHub PAT prefixes, URL credentials, sensitive query parameters, authorization forms, and high-entropy credential-like values.
- JSON, text, and top-level doctor error paths pass through redaction before emission.
- Targeted tests: doctor 55 passed; core redaction 5 passed.
- Lint, typecheck, and build passed.
- The full workspace run hit an unrelated parallel-load timing failure in unchanged
packages/orchestrator/src/index.test.ts; isolated rerun passed 17/17. - No blocking findings identified.
자세한 검토·로그: https://evidence-browser.dev0.daniworks.kr/w/github-symphony/b/project14-pr-520-20260802-063313
이 PR은 security/runtime 동작을 변경하므로 Project #14 watchdog의 low-risk Land 조건에는 해당하지 않습니다. 승인만 남기고 Project 상태는 In review로 유지합니다.
|
@codex review Land-return rework 커밋 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fba102a4c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🟡 Human review recommended
The changes adjust security-critical redaction behavior via new regex/heuristics and should receive final human review to validate false-positive/false-negative risk in real logs.
Pull request overview
This PR strengthens the Observability layer by ensuring secret redaction is applied not only to structured-key fields but also to free-form text fields (e.g., error/reason/message/stderr) and by routing all doctor outputs (JSON/text/bundle summary/top-level stderr) through the same redaction boundary, addressing leakage risks described in #442.
Changes:
- Enable recursive redaction of all string values in
redactObservabilitySecrets*, with expanded patterns for GitHub tokens, URL-embedded credentials, sensitive query params, and key-like text forms. - Add high-entropy candidate detection to redact opaque secrets that don’t match known token patterns.
- Apply the same redaction to
doctorcommand stdout JSON, stdout text, and stderr, with test coverage for JSON output and stderr leakage cases.
File summaries
| File | Description |
|---|---|
| packages/core/src/observability/redaction.ts | Expands redaction coverage to free-form strings, adds additional token/key patterns and a high-entropy redaction pass. |
| packages/core/src/observability/redaction.test.ts | Adds regression tests for free-form persisted fields, URL/query redaction, and JSON/NDJSON parse preservation. |
| packages/cli/src/commands/doctor.ts | Ensures all doctor output paths (JSON/text/stderr) are redacted immediately before writing. |
| packages/cli/src/commands/doctor.test.ts | Adds tests verifying JSON error secret redaction, stderr redaction, and bundle-path redaction behavior. |
| .changeset/redact-observability-tokens.md | Publishes a CLI patch changeset documenting the redaction enhancement for #442. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
완성도 검증 — Request Changes (핑퐁 1/6)
1. 원본 이슈(#442) 수용 조건 — ✅ 기능적으로 충족
| 완료 조건 | 상태 |
|---|---|
| free-form 텍스트 리댁션 활성화 | ✅ redactStringValues: true |
| fine-grained PAT·URL 임베드 토큰 패턴 커버 | ✅ github_pat_·gh[pousr]_·URL 자격증명·민감 query·고엔트로피 |
| doctor 출력 전체 리댁션 | ✅ JSON·text·bundle 요약·stderr 모두 공통 경로 통과 |
| TC 작성 및 통과 | ✅ 타깃 61건 |
2. Smoke Test — ✅ (로컬)
Preview 환경이 없어 로컬에서 실시했습니다.
pnpm --filter @gh-symphony/core test→ 181 passedpnpm --filter @gh-symphony/cli test→ 492 passed- 정상 경로 및 커버된 엣지 케이스 동작 확인.
3. Code Review — ❌ 블로커 2건 (인라인 참조)
자동 리뷰어(codex)가 이미 지적했고 미해결 상태로 남아 있는 두 스레드를 독립적으로 재현·확인했습니다. 둘 다 이 기능이 보호해야 할 JSON/NDJSON 아티팩트 자체를 손상시키며, 앞서 수정한 discussion_r3698077094와 동일한 구조 손상 계열입니다.
redaction.ts:160(query pass,discussion_r3698360467) — 민감 query가 URL의 마지막 파라미터로 JSON 문자열 안에 있을 때 닫는 따옴표를 삼켜 레코드가 잘림 →JSON.parse실패. 기존 TC(line 85)는 항상&safe=yes가 뒤따라 이 경계 케이스를 미검증.redaction.ts:167(quoted-value pass,discussion_r3698360468) — 이스케이프된 따옴표에서 매칭이 조기 종료되어 값 일부만 리댁션 → JSON 손상 및 시크릿 일부(def) 잔존 = 부분 누출. 보안 리댁션에서 이는 포맷 문제를 넘어 정정·보안 회귀.
재현 로그는 각 인라인 코멘트에 첨부했습니다.
결론
기능·수용 조건·happy-path는 완성되었으나, 리댁션 정규식이 특정 엣지 케이스에서 관측성 아티팩트를 손상시키고 시크릿을 부분 누출하는 회귀가 남아 있어 현재 상태로는 병합 불가합니다. 위 2건(경계 delimiter 제외 + 이스케이프 따옴표 소비)과 각 회귀 TC를 반영하면 다음 라운드에서 Approve 가능한 상태로 판단합니다. 오버 엔지니어링 없이 지적된 두 정규식 경계만 좁히면 됩니다.
Generated by Claude Code
|
@codex review Request Changes의 두 JSON/NDJSON 회귀를 커밋 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 162c910811
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Authorization JSON/string delimiter 회귀까지 커밋 @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8c9b0911be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
unquoted 시크릿의 쉼표·세미콜론 뒤 fragment 누출과 JSON 숫자·불리언·null 치환 문법 손상을 커밋 @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 970bfb426d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| redacted, | ||
| /((?:"token"|'token'|token)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/gi, | ||
| "env_token", | ||
| /((?:["'])?\b[A-Za-z0-9_.-]*(?:token|secret|api[-_.]?key|password|passwd|credential|private[-_.]?key)[A-Za-z0-9_.-]*(?:["'])?\s*[:=]\s*)(["'])((?:\\.|(?!\2)[^\\])*)\2/gi, |
There was a problem hiding this comment.
Fresh evidence beyond the prior backslash-escaped-quote case occurs when a copied WORKFLOW.md or worker log uses YAML's valid doubled-apostrophe form, such as password: 'abc''def': this matcher treats the first apostrophe after abc as the closing delimiter and emits password: '[REDACTED]''def', leaving part of the credential in the support bundle. Consume doubled quote delimiters while locating the actual end of the quoted value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정했습니다. quoted-value matcher가 같은 delimiter 두 개를 내부 escape로 먼저 소비하도록 변경해 password: 'abc''def' 전체를 실제 닫는 apostrophe까지 리댁션합니다. 출력이 password: '[REDACTED]'인지와 abc·def fragment 부재를 검증하는 회귀 TC를 추가했습니다. 커밋은 8d402fa이며 core·doctor 타깃 68건과 관련 lint/format/typecheck가 통과했습니다.
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /((?:["'])?\b[A-Za-z0-9_.-]*(?:token|secret|api[-_.]?key|password|passwd|credential|private[-_.]?key)[A-Za-z0-9_.-]*(?:["'])?\s*[:=]\s*)(?!["']|\[REDACTED\])([^\s}\]"']+)/gi, |
There was a problem hiding this comment.
When a compact worker or orchestrator log uses adjacent fields such as token=abc,status=failed or password=abc;result=denied, this value class consumes the punctuation and every subsequent field up to whitespace, producing only token=[REDACTED] and discarding useful diagnostics. Treat comma or semicolon followed by another key= field as a boundary while continuing to allow those characters inside an otherwise uninterrupted credential.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정했습니다. 일반 unquoted 값에서 쉼표·세미콜론 자체는 자격증명 일부로 계속 허용하되, 바로 뒤에 새 key=가 시작될 때만 compact log 필드 경계로 인식합니다. token=abc,status=failed와 password=abc;result=denied가 후속 필드를 보존하는 회귀 TC를 추가했습니다. 커밋은 8d402fa이며 core·doctor 타깃 68건과 관련 lint/format/typecheck가 통과했습니다.
|
YAML doubled apostrophe의 시크릿 부분 누출과 compact log의 후속 필드 과잉 삭제를 커밋 @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d402fa346
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /((?:["'])?\b[A-Za-z0-9_.-]*(?:token|secret|api[-_.]?key|password|passwd|credential|private[-_.]?key)[A-Za-z0-9_.-]*(?:["'])?\s*[:=]\s*)(?!["']|\[REDACTED\])([^\s}\]"']+?)(?=[,;](?=[A-Za-z0-9_.-]+\s*=)|\s|[}\]"']|$)/gi, |
There was a problem hiding this comment.
When createSupportBundle copies a normal turn_completed event containing "tokenUsage":{"inputTokens":20,...}, this pattern treats tokenUsage as sensitive merely because it contains token; it consumes the opening { and replaces it with the bare marker, producing invalid NDJSON such as "tokenUsage":[REDACTED]"inputTokens":.... The preceding literal pass also redacts the legitimate token-count metrics. Restrict token-key matching to actual credential keys, consistently with redactionClassForKey, so routine events.ndjson records remain machine-readable and diagnostically useful.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정했습니다. raw JSON/YAML/log assignment의 quoted·literal·unquoted 세 경로가 구조화 객체와 동일한 redactionClassForKey 판정을 사용하도록 변경했습니다. 따라서 tokenUsage·inputTokens·outputTokens·totalTokens는 보존하면서 githubGraphqlToken처럼 실제 credential suffix만 계속 리댁션합니다.
turn_completed NDJSON을 실제 JSON.parse하고 token-count 객체 전체 보존과 credential 제거를 함께 검증하는 회귀 TC를 추가했습니다. 커밋은 1e13e9d이며 core·doctor 타깃 69건과 관련 lint·format·core typecheck가 통과했습니다.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
완성도 검증 — Approve (핑퐁 2/6, 최종)
지난 라운드(핑퐁 1/6)에서 지적한 JSON/NDJSON 손상 및 부분 누출 회귀 2건이 162c910·8c9b091에서 반영되어, 이번 라운드에서 독립적으로 재현·검증했습니다.
1. 원본 이슈(#442) 수용 조건 — ✅ 전부 충족
| 완료 조건 | 상태 |
|---|---|
| free-form 텍스트 리댁션 활성화 | ✅ redactObservabilitySecretsWithStats의 redactStringValues: true |
| fine-grained PAT·URL 임베드 토큰 패턴 커버 | ✅ github_pat_·gh[pousr]_·URL 자격증명·민감 query·고엔트로피 |
| doctor 출력 전체 리댁션 | ✅ JSON·text·bundle 요약·top-level stderr 모두 writeDoctorJson/writeDoctorText 공통 경로 |
| TC 작성 및 통과 | ✅ core 9건 + doctor 55건 |
2. Smoke Test — ✅ (로컬, Preview 환경 없음)
pnpm -r build→ passredaction.test.ts→ 9 passed,doctor.test.ts→ 55 passed- 독립 적대적 재현 (컴파일된
dist에 지적 케이스 직접 투입,JSON.parse왕복 + 시크릿 조각 잔존 검사):- final query param
?access_token=…(마지막 파라미터) → parse OK, 누출 없음 - escaped quote password
"abc \"def\" rest"(이전 부분 누출) → parse OK,abc/def/rest조각 전부 제거 Authorization: Basic …in JSON → parse OK, 누출 없음- quoted
{"token":"abc def"}재처리 → parse OK, 구조 보존 github_pat_/gho_/ghs_→ 전부 리댁션- 오탐 확인: git commit SHA·
/tmp/…-bundle-…경로 정상 보존
- final query param
3. Code Review — ✅ 블로커 없음
- 인라인 리뷰 스레드 7건 전부 resolved. 이전 지적(경계 delimiter 제외, escaped quote 소비, Authorization 값 종료)이 코드에 정확히 반영됨.
- 고엔트로피 탐지는 대·소문자+숫자 동시 요구·entropy≥4·구분자≤2로 오탐을 억제했고, dead condition(
candidate.includes(REDACTED))도 제거됨. - 신규 회귀 없음.
결론
수용 조건·smoke·코드 품질 모두 통과하며, 이 기능이 보호해야 할 관측성 아티팩트를 손상시키던 회귀가 완전히 해소되었습니다. 추가 핑퐁은 불필요한 오버 엔지니어링이라 판단하여 Approve합니다.
Generated by Claude Code
|
Land-return P2를 커밋 @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e13e9d10e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| text: string, | ||
| counts: Map<RedactionClass, number> | ||
| ): string { | ||
| return text.replace(/[A-Za-z0-9+_-]{32,}={0,2}/g, (candidate) => { |
There was a problem hiding this comment.
When a free-form credential uses standard Base64 and contains /, such as Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e, this character class splits it into fragments shorter than 32 characters, so the high-entropy pass leaves the entire credential unchanged in persisted events and support bundles. Since the matcher already supports Base64 + and padding, include / as well so ordinary standard-Base64 secrets are covered.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정했습니다. 고엔트로피 후보 문자 클래스를 [A-Za-z0-9+/_-]로 확장해 표준 Base64의 / 포함 free-form credential도 하나의 후보로 탐지하고 [REDACTED]로 치환합니다. 대·소문자·숫자·entropy·separator 가드는 유지해 일반 파일 경로는 보존합니다.
회귀 TC에서 Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e 누출 방지와 /tmp/doctor-config-gMkszL/WORKFLOW.md 경로 보존을 함께 검증했습니다. 커밋은 fa1ec9b입니다.
| return matched; | ||
| } | ||
|
|
||
| incrementRedaction(counts, "secret_key"); |
There was a problem hiding this comment.
For raw assignments such as GITHUB_TOKEN=abc or OPENAI_API_KEY=abc, redactionClassForKey correctly recognizes env_token or api_key, but this helper discards that result and always records secret_key. Support-bundle manifests and doctor summaries therefore report misleading redaction-class counts, regressing the categories previously emitted by the dedicated token and API-key passes; increment the class returned by redactionClassForKey instead.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정했습니다. raw assignment matcher가 redactionClassForKey(key)의 반환값을 저장해 실제 분류를 그대로 집계하도록 변경했습니다. 따라서 GITHUB_TOKEN은 env_token, OPENAI_API_KEY는 api_key로 기록되며 값 자체는 계속 [REDACTED]입니다.
두 assignment와 class count를 검증하는 회귀 TC를 추가했습니다. 커밋은 fa1ec9b입니다.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
완성도 검증 — Request Changes (핑퐁 3/6)
지난 라운드(핑퐁 2/6) Approve 이후 커밋
1e13e9d(tokenUsage 보존)가 추가되었고, 그 커밋에 대해 codex가 미해결 신규 지적 2건을 남겨 재검증이 필요해 재오픈합니다. 승인은8c9b091기준이었고 현재 HEAD가 아닙니다.
1. 원본 이슈(#442) 수용 조건 — ✅ 4개 전부 충족 (현 HEAD 1e13e9d 기준)
| 완료 조건 | 상태 |
|---|---|
| free-form 텍스트 리댁션 활성화 | ✅ redactStringValues: true |
| fine-grained PAT·URL 임베드 토큰 패턴 커버 | ✅ github_pat_·gh[pousr]_·URL 자격증명·민감 query |
| doctor 출력 전체 리댁션 | ✅ JSON·text·bundle 요약·stderr 공통 경로 |
| TC 작성 및 통과 | ✅ core+doctor 타깃 69 passed |
2. Smoke Test — ✅ (로컬, Preview 환경 없음)
pnpm build(전 워크스페이스) → passredaction.test.ts+doctor.test.ts→ 69 passed- 적대적 재현 (컴파일된
dist에 케이스 직접 투입):github_pat_/gho_/ghs_, 명명된 provider 토큰, escaped-quote/doubled-quote, final query param, Authorization → 전부 리댁션·구조 보존 ✅- 회귀 발견:
/포함 표준 Base64 시크릿 → 완전 누출 (아래 3-①)
3. Code Review — ❌ 블로커 1건 + 🟡 non-blocking 1건 (인라인 참조)
codex가 1e13e9d에 남긴 미해결 스레드 2건을 독립적으로 재현·확인했습니다.
- ① P1 / 블로커 —
redaction.ts:232(discussion_r3699300381): 고엔트로피 클래스가/를 제외해/포함 표준 Base64 시크릿이 조각나 통과 → free-form 텍스트에서 완전 누출. 이 PR의 존재 이유인 #442 H4 사례. 재현:Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e→redactions:[],/없는 동일 값은 리댁션됨. - ② P2 / non-blocking —
redaction.ts:220(discussion_r3699300384):redactionClassForKey결과를 버리고 항상secret_key집계 → bundle/doctor 카운트 오분류. 누출 아님(관측성 정확도).
결론
수용 조건·happy-path·smoke는 완성이나, 보안 리댁션 기능에 일반적 인코딩(표준 Base64) 시크릿의 완전 누출 회귀가 현 HEAD에 남아 있어 병합 불가입니다. 수정 범위는 문자 클래스에 / 1자 추가 + 오탐 가드 회귀 TC(①)와 클래스 집계 1줄(②)로 오버 엔지니어링이 아닙니다. ① 반영 시 다음 라운드에서 Approve하겠습니다(②는 함께 처리 권장이나 단독으로는 승인 보류 사유 아님).
Generated by Claude Code
| text: string, | ||
| counts: Map<RedactionClass, number> | ||
| ): string { | ||
| return text.replace(/[A-Za-z0-9+_-]{32,}={0,2}/g, (candidate) => { |
There was a problem hiding this comment.
Blocking (P1) — confirms open thread discussion_r3699300381. 고엔트로피 문자 클래스 [A-Za-z0-9+_-]가 /를 포함하지 않아, /를 담은 표준 Base64 시크릿이 32자 미만 조각으로 쪼개져 어떤 pass에도 걸리지 않고 그대로 통과합니다. 이는 이 PR이 막으려는 #442 H4(free-form 텍스트 토큰 누출)의 정확한 사례입니다.
컴파일된 dist에 직접 투입해 재현했습니다:
IN : error: token Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e failed
OUT: error: token Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e failed → redactions:[] ❌ 완전 누출
(대조군, '/' 없는 동일 값) → error: token [REDACTED] failed → ✅ 리댁션됨
+·padding은 이미 지원하므로 표준 Base64의 /도 포함해 주세요 ([A-Za-z0-9+/_-]). 다만 / 추가는 긴 파일 경로의 오탐 위험을 키우므로, 경로가 과다 리댁션되지 않는지 확인하는 회귀 TC를 함께 넣어 false-positive 가드(대·소문자+숫자 동시 요구·entropy≥4·구분자 제한)가 유지되는지 방어해 주세요.
Generated by Claude Code
There was a problem hiding this comment.
수정했습니다. 고엔트로피 후보 문자 클래스를 [A-Za-z0-9+/_-]로 확장해 표준 Base64의 / 포함 free-form credential도 하나의 후보로 탐지하고 [REDACTED]로 치환합니다. 대·소문자·숫자·entropy·separator 가드는 유지해 일반 파일 경로는 보존합니다.
회귀 TC에서 Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e 누출 방지와 /tmp/doctor-config-gMkszL/WORKFLOW.md 경로 보존을 함께 검증했습니다. 커밋은 fa1ec9b입니다.
| return matched; | ||
| } | ||
|
|
||
| incrementRedaction(counts, "secret_key"); |
There was a problem hiding this comment.
Non-blocking (P2) — confirms open thread discussion_r3699300384. 바로 위 redactionClassForKey(key)가 env_token/api_key/authorization_header를 정확히 판정하는데도 여기서 결과를 버리고 항상 secret_key로 집계합니다. 그 결과 support-bundle 매니페스트·doctor 요약의 redaction-class 카운트가 실제와 어긋나, 전용 token/api-key pass가 내보내던 분류를 회귀시킵니다.
재현 (free-form 할당):
GITHUB_TOKEN=abcdef → [{"class":"secret_key",...}] (기대: env_token)
OPENAI_API_KEY=abcdef → [{"class":"secret_key",...}] (기대: api_key)
값 자체는 정상 리댁션되므로 누출은 아니며 관측성 정확도 문제입니다. redactionClassForKey(key)의 반환 클래스를 그대로 incrementRedaction에 넘겨주면 됩니다. 블로커는 아니니 P1과 함께 처리해 주시면 좋겠습니다.
Generated by Claude Code
There was a problem hiding this comment.
수정했습니다. raw assignment matcher가 redactionClassForKey(key)의 반환값을 저장해 실제 분류를 그대로 집계하도록 변경했습니다. 따라서 GITHUB_TOKEN은 env_token, OPENAI_API_KEY는 api_key로 기록되며 값 자체는 계속 [REDACTED]입니다.
두 assignment와 class count를 검증하는 회귀 TC를 추가했습니다. 커밋은 fa1ec9b입니다.
|
@codex review Cycle 10 Request Changes를 반영했습니다. P1/P2 inline comment 네 건에 답변했고, 타깃 71건·workspace 전체 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fa1ec9b708
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| text: string, | ||
| counts: Map<RedactionClass, number> | ||
| ): string { | ||
| return text.replace(/[A-Za-z0-9+/_-]{32,}={0,2}/g, (candidate) => { |
There was a problem hiding this comment.
When a normal workspace path has mixed case and digits but no more than two _/- characters, the slash-inclusive candidate spans path segments and passes every entropy guard; for example, .runtime/projects/tenant-a/ENG-123/repository is replaced inside the string. Because redactObservabilitySecrets now scans all strings and OrchestratorFsStore.saveRun applies it to the complete run record, the persisted workingDirectory becomes a path containing [REDACTED]; the next reconciliation then passes that invalid path to the after_run hook as its cwd and exposes the corrupted path through status surfaces. Detect Base64 slashes without treating multi-segment filesystem paths as opaque credentials.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (869af2c). redactHighEntropyValues가 match offset/source를 사용해 다중 세그먼트 filesystem path를 path boundary 또는 dot-prefix 문맥에서 entropy 후보로 처리하지 않도록 했고, slash가 포함된 opaque Base64 후보 redaction은 유지했습니다. .runtime/projects/tenant-a/ENG-123/repository 회귀 TC를 추가했으며 타깃·workspace 검증이 통과했습니다.
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /((?:["'])?\b([A-Za-z0-9_.-]+)(?:["'])?\s*[:=]\s*)(?!["']|\[REDACTED\])([^\s}\]"']+?)(?=[,;](?=[A-Za-z0-9_.-]+\s*=)|\s|[}\]"']|$)/gi, |
There was a problem hiding this comment.
Fresh evidence beyond the prior comma/semicolon case: an unquoted environment or log value such as GITHUB_TOKEN=abc}def was fully covered by the former non-whitespace token pass, but this value class stops before } and emits GITHUB_TOKEN=[REDACTED]}def, leaking the credential suffix. The JSON delimiter exclusions need to apply only in JSON context, while raw KEY=value assignments must still consume the complete non-whitespace value.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (869af2c). raw = assignment를 JSON/YAML : matcher와 분리해 non-whitespace 값 전체를 소비하도록 했으므로 GITHUB_TOKEN=abc}def의 def suffix가 더 이상 남지 않습니다. compact log의 ,key=·;key= 경계 보존은 유지하고 }·] delimiter 회귀 TC를 추가했으며 타깃·workspace 검증이 통과했습니다.
|
Cycle 11 rework is complete on
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 869af2c198
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /((?:["'])?\b([A-Za-z0-9_.-]+)(?:["'])?\s*:\s*)(?!["']|\[REDACTED\])([^\s}\]"']+?)(?=[,;](?=[A-Za-z0-9_.-]+\s*=)|\s|[}\]"']|$)/gi, |
There was a problem hiding this comment.
When a copied YAML artifact contains a multiword plain scalar such as password: correct horse battery staple, this matcher treats the first whitespace as the value boundary and emits password: [REDACTED] horse battery staple. The support bundle therefore leaks most of a passphrase; consume the complete YAML plain scalar up to its actual line/comment boundary rather than stopping at the first space.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (09151bb). YAML plain scalar matcher가 민감 키의 값을 첫 공백에서 끊지 않고 줄 끝 또는 인라인 주석 경계까지 소비한 뒤 [REDACTED]로 치환합니다. quoted YAML과 JSON은 기존 구조 보존 경로를 유지합니다. password: correct horse battery staple # diagnostic 회귀 TC를 추가했고, core 18건·doctor 포함 73건 및 전체 lint/test/typecheck/build가 통과했습니다.
| if (candidate.startsWith("/") || preceding === ".") { | ||
| return true; |
There was a problem hiding this comment.
Fresh evidence beyond the earlier slash-credential thread occurs when a valid high-entropy Base64 credential begins with / and contains another slash, such as /Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e: it has three segments, so this branch classifies it as an absolute filesystem path and returns it unchanged before the entropy checks. Such credentials remain exposed in persisted events and support bundles; path exclusion needs additional path context rather than relying on a leading slash alone.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (09151bb). entropy path guard가 선행 /만으로 absolute path로 분류하지 않도록 바꿔 slash-prefixed opaque Base64 credential은 entropy matcher에서 redaction되고, /tmp·/Users 같은 알려진 filesystem root와 명시적 path 문맥은 보존됩니다. /Ab3dEf5hIj7kLm9nOp1q/Rs3tUv5wXy7zA9bCd2e 회귀 TC를 추가했으며 전체 검증이 통과했습니다.
| return true; | ||
| } | ||
|
|
||
| return segments.some((segment) => /[-_]/.test(segment)); |
There was a problem hiding this comment.
When observability text contains a relative multi-segment path such as runtime/projects/tenantA/ENG123/repository, this fallback does not recognize it as a path because none of its segments contain _ or -; the candidate satisfies the mixed-case, digit, length, and entropy guards and is replaced with [REDACTED]. This removes legitimate workspace/path diagnostics, including the relative paths permitted by the upstream workspace model, so multi-segment path detection cannot depend on dashed segments.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (09151bb). filesystem path 판정이 -/_ 세그먼트 존재 여부에 의존하지 않도록 알려진 workspace root와 path 문맥을 사용합니다. runtime/projects/tenantA/ENG123/repository 상대 경로가 보존되는 회귀 TC를 추가했고, 기존 .runtime·absolute path 보호도 유지한 채 전체 검증을 통과했습니다.
|
Cycle 12 rework is complete on
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09151bbf11
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "workspace", | ||
| "workspaces", | ||
| ]); | ||
| if (knownPathRoot.has(firstSegment)) { |
There was a problem hiding this comment.
Fresh evidence beyond the earlier absolute-path thread: when the runtime is configured under a valid but non-whitelisted root such as /data/workspaces, a path like /data/workspaces/tenantA/ENG123/repository satisfies the entropy guards and is replaced with [REDACTED] because data is absent from this set and a structured workingDirectory has no textual path context. OrchestratorFsStore.saveRun persists that transformed path, and reconciliation later uses it as the after_run cwd, so deployments outside the enumerated roots get corrupted run state; recognize configured/absolute paths without relying on a fixed root list.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (07ea889). 고정된 filesystem root 목록을 제거하고, absolute/relative 후보의 readable path segment와 기존 path 문맥을 사용해 임의 root 아래의 경로를 보존하도록 변경했습니다. 구조화된 workingDirectory: "/data/workspaces/tenantA/ENG123/repository" 회귀 TC를 추가했으며, slash-prefixed opaque Base64 credential은 계속 [REDACTED] 처리됩니다. core 전체 195건과 workspace lint/test가 통과했습니다.
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /(^|\r?\n)([ \t]*)(-\s+)?([A-Za-z0-9_.-]+)(\s*:\s+)(?!["']|\[REDACTED\])([^\r\n]*?\S)([ \t]+#.*)?(?=\r?\n|$)/gm, |
There was a problem hiding this comment.
When copied YAML uses a block scalar such as password: |- followed by an indented correct horse battery staple, this matcher replaces only the |- indicator and leaves the entire indented secret unchanged. createSupportBundle copies WORKFLOW.md through this text redactor, so multiline credentials or passphrases remain disclosed in the resulting bundle; detect block-scalar headers and redact their indented payload through the YAML indentation boundary.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (07ea889). 민감 YAML key의 |/> block-scalar header를 인식하고, header indentation보다 깊은 payload를 다음 sibling 경계까지 함께 소비한 뒤 password: [REDACTED]로 치환합니다. 중첩 YAML sibling 보존과 다중 단어 multiline payload 누출 방지 회귀 TC를 추가했으며, core 전체 195건과 workspace lint/test가 통과했습니다.
|
Cycle 13 rework is complete on
@codex review |
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
완성도 검증 — Approve (핑퐁 4/6, 최종)
지난 라운드(핑퐁 3/6)의 P1 블로커(/ 포함 표준 Base64 시크릿 완전 누출)와 이후 codex가 남긴 신규 edge case들이 fa1ec9b·869af2c·09151bb·07ea889에서 반영되어, 현 HEAD 07ea889에서 독립적으로 재현·검증했습니다.
1. 원본 이슈(#442) 수용 조건 — ✅ 4개 전부 충족
| 완료 조건 | 상태 |
|---|---|
| free-form 텍스트 리댁션 활성화 | ✅ redactObservabilitySecretsWithStats가 redactStringValues: true로 모든 string value 재귀 처리 |
| fine-grained PAT·URL 임베드 토큰 패턴 커버 | ✅ github_pat_·gh[pousr]_·sk-·lin_·URL 자격증명·민감 query·고엔트로피(표준 Base64 / 포함) |
| doctor 출력 전체 리댁션 | ✅ JSON·text·bundle 요약·top-level stderr 공통 경로 |
| TC 작성 및 통과 | ✅ core 195 + doctor 55 |
2. Smoke Test — ✅ (로컬, Preview 환경 없음)
pnpm --filter @gh-symphony/core build→ passpnpm --filter @gh-symphony/core test→ 195 passed /doctor.test.ts→ 55 passed- 독립 적대적 재현 (컴파일된
dist에 케이스 직접 투입,JSON.parse왕복 + 시크릿 조각 잔존 검사, 총 37건):- 토큰 패턴:
github_pat_·gho_·ghs_·sk-·URL 임베드 credential → 전부 리댁션 - 구조 보존: final query param, escaped-quote, doubled-quote(YAML),
Authorization: Basic …in JSON, numeric literal → parse OK + 조각 완전 제거 - YAML: plain multiword scalar, block scalar(
|-) multiline payload → 누출 없음, sibling 필드 보존 - 고엔트로피:
/포함·slash-prefixed 표준 Base64 → 리댁션 (지난 라운드 블로커 해소 확인) - compact/raw 할당:
KEY=abc,def·KEY=abc}defsuffix 누출 없음,token=…,status=failed후속 필드 보존 - 오탐 가드: git commit SHA, 임의 root absolute path(
/data/workspaces/…), dash 없는 relative path,/tmp/…bundle path,tokenUsage메트릭 전부 보존 - 구조화 경로: 중첩 객체·배열 내부 토큰 리댁션, 실제 multiline stderr blob의
ghp_/X-API-Key제거 +workingDirectory경로 보존
- 토큰 패턴:
- CI
Test·Container Smoke→ green @07ea889
3. Code Review — ✅ 블로커 없음
- 인라인 리뷰 스레드 23건 전부 코드에 반영 확인. 지난 라운드 블로커(고엔트로피 문자 클래스
/미포함)는[A-Za-z0-9+/_-]확장으로 해소되었고, path 오탐은 고정 root 목록 제거 + readable segment/path 문맥 판정으로 방어됨. - 신규 회귀 없음.
- (ℹ️ non-blocking, 조치 불요) 고엔트로피 pass는 오탐 억제를 위해 대·소문자+숫자 동시 요구·entropy≥4를 강제하므로, key 문맥이 전혀 없는 순수 소문자 opaque 값은 탐지 대상이 아닙니다. 이는 #442(알려진 토큰 패턴·free-form 누출) 범위 밖의 엔트로피 탐지 본질적 trade-off이며 회귀가 아닙니다. 문자 클래스를 더 넓히면 경로/산문 오탐이 급증하므로 현 상태 유지가 옳습니다.
결론
수용 조건·smoke·코드 품질 모두 통과하며, 이 기능이 보호해야 할 관측성 아티팩트를 손상시키던 회귀가 완전히 해소되었습니다. 남은 항목은 엔트로피 탐지의 본질적 한계(오탐 방지 trade-off)뿐이며, 이를 더 좁히는 것은 오버 엔지니어링입니다. 추가 핑퐁 없이 Approve합니다.
Generated by Claude Code
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 07ea8890ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ); | ||
| redacted = replaceSensitiveKeyAndCount( | ||
| redacted, | ||
| /((?:["'])?\b([A-Za-z0-9_.-]+)(?:["'])?\s*:\s*)(?!["']|\[REDACTED\])([^\s}\]"']+?)(?=[,;](?=[A-Za-z0-9_.-]+\s*=)|\s|[}\]"']|$)/gi, |
There was a problem hiding this comment.
Fresh evidence beyond the resolved tokenUsage case occurs when a copied NDJSON or worker-log record has an actually sensitive object or array, such as {"credentials":{"value":"abc"},"status":"failed"}: this scalar matcher replaces only the opening {, yielding malformed JSON while leaving abc exposed. Handle balanced container values—or parse each JSON record—so the complete sensitive value is removed without destroying delimiters.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (672a77b). redactSensitiveJsonContainers가 민감 JSON key 뒤의 balanced object/array를 문자열 [REDACTED]로 통째로 소비하고, escaped quote를 고려해 JSON 구조를 보존합니다. {"credentials":{"value":"container-secret"}}와 array container를 JSON.parse로 왕복하는 회귀 TC를 추가했고 core 199건이 통과했습니다.
| if (pathSegments.some(isReadableFilesystemSegment)) { | ||
| return true; |
There was a problem hiding this comment.
Fresh evidence beyond the resolved slash-prefixed sample is a valid standard-Base64 credential such as /abc/DEF/Gh1jKl3mNo5pQr7sTu9vWxYzAbCdEfG: because one slash-delimited fragment is the readable segment abc, this some(...) check classifies the entire token as a filesystem path and preserves it in persisted events and support bundles. Path recognition needs stronger path context than the presence of any short alphabetic segment.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (672a77b). filesystem path 판정이 짧은 readable segment 하나만으로 보존하지 않도록 강화되어, 경로 문맥·점이 있는 파일명·3개 이상의 readable path segment가 있을 때만 경로로 보존합니다. /abc/DEF/Gh1jKl3mNo5pQr7sTu9vWxYzAbCdEfG 회귀 TC는 [REDACTED]를 확인하고, 기존 arbitrary root/workspace path TC도 유지합니다.
|
|
||
| for (let index = 0; index < lines.length; index += 1) { | ||
| const match = | ||
| /^([ \t]*)(-\s+)?([A-Za-z0-9_.-]+)(\s*:\s+)(?!["']|\[REDACTED\])([|>])(?:[-+]?\d?)([ \t]+#.*)?$/.exec( |
There was a problem hiding this comment.
Fresh evidence beyond the resolved |- case occurs with YAML's valid digit-before-chomping form, for example password: |2- followed by a two-space-indented secret. This expression accepts |-2 but not |2-, so the later plain-scalar pass redacts only the header while leaving every payload line visible in the support bundle; recognize both legal modifier orders before scalar redaction.
AGENTS.md reference: AGENTS.md:L5-L7
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (672a77b). YAML block-scalar modifier가 chomping-first(|-2)와 indentation-first(|2-) 순서를 모두 인식하도록 보강되었습니다. 두 형식의 multiline payload가 남지 않는 회귀 TC를 추가했습니다.
| continue; | ||
| } | ||
|
|
||
| const headerIndent = match[1].length; |
There was a problem hiding this comment.
When a block scalar is an inline sequence mapping entry, such as - password: |- followed by an indented status: failed sibling, match[1] is empty because the - prefix is captured separately. The boundary is therefore treated as column zero, and the loop removes status along with the secret payload; include the sequence-marker width when calculating the mapping entry's effective indentation.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (672a77b). sequence mapping의 effective indentation을 mapping indent와 - marker 폭의 합으로 계산해, - password: |- payload만 제거하고 같은 sequence의 status sibling은 보존합니다. sibling 보존 회귀 TC를 추가했습니다.
|
Cycle 15 rework is complete on
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 672a77b498
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const pathSegments = candidate.startsWith("/") ? segments.slice(1) : segments; | ||
| const readableSegments = pathSegments.filter(isReadableFilesystemSegment); | ||
| if ( | ||
| readableSegments.length >= 3 || |
There was a problem hiding this comment.
When a standard Base64 credential contains three short alphabetic slash-delimited fragments, such as the canonical value abc/DEF/ghi/Jk1Lm3No5Pq7Rs9TuVwX, this condition classifies it as a filesystem path even in free-form text like upstream credential ..., so redactHighEntropyValues leaves the complete secret visible. Fresh evidence beyond the resolved single-readable-segment thread is that increasing the threshold to three still exempts valid Base64 without any path context; require actual path context before preserving such candidates.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (8064ae8). readable slash-delimited candidate를 filesystem path로 보존하는 조건에 실제 path context를 추가해, message: upstream credential abc/DEF/ghi/Jk1Lm3No5Pq7Rs9TuVwX는 [REDACTED] 처리됩니다. workingDirectory·safePath 같은 구조화 path 필드와 working directory·artifact at 문맥의 정상 경로 보존은 유지합니다. 해당 누출/오탐 회귀 TC와 core 전체 201 tests가 통과했습니다.
| redacted, | ||
| /((?:"apiKey"|'apiKey'|apiKey)\s*:\s*)(?:"([^"]*)"|'([^']*)'|([^\s,}\]]+))/g, | ||
| "api_key", | ||
| /(^|\r?\n)([ \t]*)(-\s+)?([A-Za-z0-9_.-]+)(\s*:\s+)(?!["']|\[REDACTED\])([^\r\n]*?\S)([ \t]+#.*)?(?=\r?\n|$)/gm, |
There was a problem hiding this comment.
When a copied YAML artifact uses a sensitive key with a block mapping or sequence, such as credentials: followed by two indented items, the \s+ after the colon crosses the newline and this scalar matcher consumes only the first child. The output leaves later credential entries exposed and can be malformed YAML; for example, a two-item credential sequence becomes credentials:\n [REDACTED]\n - second-secret. Detect sensitive YAML containers and consume their full indentation-delimited value instead of treating the first child as a plain scalar.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
수정 완료했습니다 (8064ae8). 민감 YAML key가 값 없이 끝난 뒤 indented mapping/sequence를 시작하면 sibling indentation까지 전체 payload를 소비하고 key: [REDACTED]로 치환합니다. credentials:의 두 sequence 항목과 password:의 두 mapping child가 모두 제거되고 status·result sibling은 보존되는 회귀 TC를 추가했으며 core 전체 201 tests가 통과했습니다.
hojinzs
left a comment
hojinzs
left a comment
There was a problem hiding this comment.
완성도 검증 — Approve (핑퐁 4/6, 최종)
핑퐁 3/6에서 지적한 P1(표준 Base64 / 완전 누출)을 포함해, 이후 codex 사이클(10–15)에서 제기된 엣지 케이스가 현 HEAD 8064ae8까지 전부 반영되었습니다. 이번 라운드는 현 HEAD를 독립적으로 빌드·재현·검증한 결과입니다.
1. 원본 이슈(#442) 수용 조건 — ✅ 4/4 충족
| 완료 조건 | 상태 |
|---|---|
| free-form 텍스트 리댁션 활성화 | ✅ redactStringValues: true |
| fine-grained PAT·URL 임베드 토큰 패턴 커버 | ✅ github_pat_·gh[pousr]_·URL 자격증명·민감 query·고엔트로피 |
| doctor 출력 전체 리댁션 | ✅ JSON·text·bundle 요약·top-level stderr 공통 writeDoctorJson/writeDoctorText 경로 |
| TC 작성 및 통과 | ✅ core 26건 + doctor 55건 |
2. Smoke Test — ✅ (로컬, Preview 환경 없음)
pnpm -r build→ pass, CITest·Container Smoke→ green @8064ae8redaction.test.ts→ 26 passed,doctor.test.ts→ 55 passed- 독립 적대적 재현(컴파일된
dist에 케이스 직접 투입,JSON.parse왕복 + 시크릿 조각 잔존 검사) → 13/13 pass:- #442 H4: free-form의
github_pat_/gho_/URL 자격증명 → 전부 리댁션 - 최신 P1
r3701004551(YAML mapping/sequence container):credentials:항목 전체 제거 +statussibling 보존 - 최신 P1
r3701004548(readable Base64 path-context):abc/DEF/ghi/…free-form 리댁션 + 구조화workingDirectory경로 보존 - final query param / escaped-quote password / JSON container → parse OK, 누출 없음
- 오탐 가드: git commit SHA·
/tmp/…/WORKFLOW.md경로 정상 보존
- #442 H4: free-form의
3. Code Review — ✅ 블로커 없음
- 리뷰 스레드 29건 전부 rework 커밋(
4fba102…8064ae8)으로 대응 완료. 신규 미해결 블로커 없음. - 리댁션 커버리지 91.5%(branch 82.7%), 각 수정마다 회귀 TC 동반.
결론 — 오버 엔지니어링 방지를 위한 최종 판정
수용 조건·smoke·코드 품질 모두 통과하며, #442가 요구한 free-form 토큰 누출 차단이 실증적으로 달성되었습니다.
이 기능은 정규식/엔트로피 휴리스틱 기반 시크릿 스캐너로, 적대적 YAML/Base64 입력을 무한히 합성할 수 있는 경계가 없는(unbounded) 대상입니다. 수용 조건은 사이클 2에서 이미 충족되었고 이후 사이클은 전부 방어심화(defense-in-depth) 성격의 미세 엣지 케이스였습니다. 현 상태는 실사용 위협 모델(persisted events·doctor·support bundle의 알려진 토큰·URL·고엔트로피·컨테이너 누출)을 충분히 커버하며, 추가 라운드는 한계효용이 급감하는 오버 엔지니어링이라 판단합니다. 남은 이론적 엣지 케이스는 필요 시 별도 follow-up 이슈로 분리 권장하고, 본 PR은 Approve·머지 진행을 권고합니다.
Generated by Claude Code
TL;DR
doctor출력 전체에 공통 시크릿 리댁션을 적용합니다.변경 지점 다이어그램
여기부터 보세요
packages/core/src/observability/redaction.tsredactStringValues: true적용github_pat_,gho_,ghs_, URL credential/query, custom key 및 high-entropy 토큰 탐지"[REDACTED]"로 치환해 JSON 구조 보존packages/core/src/observability/redaction.test.tspackages/cli/src/commands/doctor.ts검증
pnpm lint— passpnpm test— pass (core 201, orchestrator 223, worker 146, CLI 492 포함)pnpm typecheck— passpnpm build— passpnpm exec vitest run packages/core/src/observability/redaction.test.ts— pass, 26 testsTest,Container Smoke— pass at8064ae8위험 & 롤백
[REDACTED]로 축약하므로 진단 원문이 보존되지 않습니다. 보안상 의도된 동작입니다.8064ae8부터 이 PR의 관측성 redaction commits를 revert하면 됩니다.변경 파일
.changeset/redact-observability-tokens.mdpackages/core/src/observability/redaction.tspackages/core/src/observability/redaction.test.tspackages/cli/src/commands/doctor.tspackages/cli/src/commands/doctor.test.tsIssues — Closed #442
Fixes #442
머지 후/사람 확인
8064ae8인지 확인해 주세요.doctor --json및 support bundle이 token 조각을 남기지 않고 JSON/YAML 구조를 보존하는지 확인해 주세요.