test(support): bind support-bundle counts and secret redaction - #134
seonghobae wants to merge 8 commits into
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough지원 번들은 인증 설정과 자격 증명 출처를 검증합니다. 테스트는 번들의 런타임 카운트를 KPI 및 증거 매니페스트와 비교합니다. 직렬화된 JSON에 관리자 토큰 헤더 이름이나 토큰 값이 포함되지 않는지도 확인합니다. Changes지원 번들 검증
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Other Merge Risk: ⚪ Minimal · up to The reviewed changes strengthen support-bundle tests and introduce no identified behavior risk. Confirm required checks against this revision through the normal merge process. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
src/lib.rs— Rust package behavior
Changed behavior
classDiagram
class AppState
class SocLlmConfig
class ClearfolioConfig
class seeded
class load
class with_kev_catalog_url
class with_max_body_size
class with_clearfolio
Changed API
AppStateSocLlmConfigClearfolioConfigseededloadwith_kev_catalog_urlwith_max_body_sizewith_clearfoliowith_soc_llmwith_rate_limitwith_admin_tokenswith_credentials_sourceAppConfigmemorySupportBundleHealthStatusbuild_appexport_events_ndjsonupstream_targetAdminPrincipalparse_admin_tokensparse_event_limitparse_u32_envparse_u64_envrun_from_env
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
ce58b6ec2968f314ea223ddb9ff7228fc4222e44 - Workflow run: 34170689654
- Workflow attempt: 1
- Coverage gate:
failure
Review outcome
Coverage is a gate, not the review. This body reviews the changed product files.
Changed-File Evidence Map
classDiagram
class AppState
class SocLlmConfig
class ClearfolioConfig
class seeded
class load
class with_kev_catalog_url
class with_max_body_size
class with_clearfolio
OpenCode Review Overview
Coverage evidence did not pass, so approval is blocked. The formal pull-request review is the source-backed diff review, not this status comment. |
…ression chore(restack): adopt protected auth baseline into support-bundle regression
The commercial support-bundle test now uses a distinct admin token and checks that health reports auth as configured while the serialized bundle omits the token and the header name. Direct AppConfig loads stay source none.
There was a problem hiding this comment.
Noema LLM review
The changes improve the test suite by replacing hardcoded secrets with a variable and adding comprehensive assertions to verify the correctness of support bundle counts and the effectiveness of secret redaction during serialization. The implementation maintains consistency across API requests and ensures that sensitive authentication tokens are not leaked in the diagnostic output.
Reviewed changed lines
src/lib.rs:4811 (RIGHT): Replacing the hardcoded 'secret' with a variableadmin_tokenensures consistency across the test case setup and subsequent API requests.src/lib.rs:5025 (RIGHT): New assertions verify that support bundle counts are synchronized acrosskpisandevidence_manifest.runtime_counts, ensuring data integrity in diagnostic reports.src/lib.rs:5064 (RIGHT): The assertions confirm that neither the sensitive token value nor thex-admin-tokenkey are present in the serialized JSON, validating the redaction logic.
Adversarial validation
src/lib.rs:4811 (RIGHT)falsified: The replacement of the hardcoded 'secret' string with a variableadmin_tokenacross the test case introduces inconsistency or breakage in API request authorization. — The variableadmin_tokenis defined at line 4811 and consistently used in alljson_requestcalls (lines 4862, 4875, 4902, 4915).src/lib.rs:5025 (RIGHT)falsified: The new assertions for support-bundle counts (lines 5025-5063) might be comparing incompatible types or incorrectly mapping fields fromkpisandevidence_manifest. — The assertions correctly verify that the top-level summary counts in the support bundle are synchronized with both thekpissnapshot and theevidence_manifest.runtime_counts.src/lib.rs:5064 (RIGHT)falsified: The redaction assertions (lines 5064-5065) are insufficient to prove that sensitive credentials are not leaked in the serialized output. — The test asserts the absence of both the keyx-admin-tokenand the specific value of theadmin_token, confirming effective redaction for the configured secret.- Residual risk: none
Findings
- No blocking findings.
- Result: APPROVE
- Head SHA:
b1758bb838c7b315cdf4e55064985c3626f52e5e - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
Shared-owner RCA and repairThe exact-head Strix failure at Root cause is owned by the reusable workflow in Owner repair: ContextualWisdomLab/.github#2540, stacked on .github#2532. It exchanges GitHub Actions OIDC for a short-lived repository-scoped OpenCode GitHub App token and removes the invalid consumer-token fallback. Exact owner-tree verification: 5162 passed, 10 skipped, 40 subtests passed. Do not rerun the unchanged predecessor evidence. After the owner stack merges, revalidate this PR's then-current exact head; acceptance requires successful token exchange, central continuation dispatch, and a fresh terminal Strix verdict. |
Problem and bounded delta
Protected
mainalready carries the shutdown-listener race repair, so the unique delta on this branch is the support-bundle regression contract only:This is test-only hardening in
src/lib.rs; it does not change production runtime behavior.Protected-main adoption — refreshed 2026-09-11 KST
Protected/default
mainis exactf8260f1e03836039ff9463dd99fa982e4e270c4b. Reverse-direction restack #314 adopted that protected auth/security baseline normally intocodex/support-bundle-regression-coverage, without force or destructive rebase, producing current exact headb6c1f2cfb9d05f36b5ef14c7be6004c00af762d4. Fresh comparison remains a single-file 38-line test delta insrc/lib.rs; protected #155 behavior is inherited rather than copied.All predecessor workflow/review conclusions are historical after #314.
Exact-current evidence
On unchanged exact
b6c1f2cfb9d05f36b5ef14c7be6004c00af762d4all principal repository/security lanes are now terminal:34572688789— SUCCESS;34572688651— SUCCESS;34572688741— SUCCESS;34572688686— SUCCESS;34572688797— FAILURE at the delegated current-head terminal-settlement boundary.The earlier queued snapshot is superseded.
.github#1929owns the central CodeQL settlement defect; current repair successor.github#2040remains mutable owner-path work rather than Wardnet dependency authority. Wardnet does not add no-op commits, copy central workflows, promote predecessor verdicts, or use routine/guarded bypass while a required workflow is RED.Because required CodeQL remains non-passing, this PR stays Draft. Live ruleset
18156473also retains the generic solo-maintainer approval defect tracked by.github#772; self/model approval and routine administrator bypass remain forbidden.Keep Draft until one unchanged exact head has terminal-valid deterministic/security/CodeQL/coverage/package/SBOM/provenance/review/thread/governance evidence and fresh protected-base compatibility. No gate weakening, force push, destructive rebase, mutable foreign dependency, source copy, cross-service SQL, or predecessor-evidence transfer.
Summary by CodeRabbit