fix(privacy): redact temporal analysis logs - #1055
seonghobae wants to merge 12 commits into
Conversation
📝 WalkthroughWalkthrough로컬 오디오 분석이 템포 안정성과 지속적 템포 변화를 생성합니다. 콘텐츠 지문 기반 캐시와 검증된 상태 계약을 추가하고, 데스크톱 안내 및 차트 요약 JSON에 결과를 전달합니다. Changes템포 안정성 분석 및 전달
Estimated code review effort: 4 (Complex) | ~60 minutes Suggested reviewers: Merge Risk: 🟡 Moderate · up to Changed or unreadable local audio can return cached analysis for different content. Cache reuse should be disabled when content identity cannot be verified before merge. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
|
@opencode-agent review |
|
Protected-gate evidence for current head b11b1e0:\n\n- deterministic, security, SBOM, coverage, build, release, macOS, and Windows checks pass.\n- Strix run 33243678538 completed at 2026-08-29T08:52:28Z with failure; job 99077269073 reached the quick scan step and uploaded artifact strix-reports.\n- The artifact contains three bounded attempts against contextual-orchestrator/openai/orchestrator/free, each returning HTTP 500 internal_error; no vulnerability report was produced. The workflow explicitly fails closed when only log markers exist.\n- opencode-review is also failure because no current-head formal verdict was available.\n- No independent qualifying approval exists; merge remains blocked. I am not bypassing required gates or self-approving. |
|
Queued @opencode-agent for PR #1055 at head |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode reviewed the current-head product diff. Coverage is a separate gate.
Changed files
CHANGELOG.md— repository behaviorservices/analysis-engine/src/bandscope_analysis/temporal/analyzer.py— Python module behaviorservices/analysis-engine/tests/test_temporal.py— regression suite
Changed behavior
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["Python: analyzer.py"]
S2 --> I2["Python module behavior"]
I2 --> R2["Review risk: Python: analyzer.py"]
R2 --> V2["pytest plus coverage"]
Evidence --> S3["Test: test_temporal.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_temporal.py"]
R3 --> V3["targeted test run"]
Findings
No source-backed product finding is synthesized from the coverage gate. A coverage miss belongs in the status comment.
- Head SHA:
b11b1e0c1756921c64524d691fb4fac440abc65b - Workflow run: 33244973878
- 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
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Repository file: CHANGELOG.md"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Repository file: CHANGELOG.md"]
R1 --> V1["required checks"]
Evidence --> S2["Python: analyzer.py"]
S2 --> I2["Python module behavior"]
I2 --> R2["Review risk: Python: analyzer.py"]
R2 --> V2["pytest plus coverage"]
Evidence --> S3["Test: test_temporal.py"]
S3 --> I3["regression suite"]
I3 --> R3["Review risk: Test: test_temporal.py"]
R3 --> V3["targeted test run"]
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. |
|
@opencode-agent review Fresh exact-head re-dispatch requested for |
…ty-cue feat(tempo): surface tempo movement rehearsal cues
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
Dependency-owner handoff for the current exact BandScope head Required Strix job
The trusted wrapper intentionally supplies the same-job contextual-orchestrator loopback endpoint as Owner-side acceptance test: the central Strix integration must admit only the explicitly trusted loopback contextual-orchestrator gateway while retaining HTTPS-only validation for non-loopback/external configured endpoints, then a fresh exact-head BandScope Strix run must proceed through an authoritative scan rather than failing during endpoint admission. Until that owner-side contract is repaired and this unchanged head is rerun, this Strix result remains non-passing dependency evidence, not a BandScope vulnerability finding. |
|
@opencode-agent review Fresh exact-head re-dispatch for |
|
@opencode-agent review Central causal repair |
|
Resource Admission / single-writer finding from the live #866 sweep: this branch currently implements cache/source-content authority inside the temporal/privacy lane ( There is also a concrete resource bypass in the current fingerprint implementation: Please keep this PR Draft and preserve its unique temporal/privacy/tempo evidence, but do not land the fingerprint/cache-authority implementation as-is. Move the semantic requirement — cache keys and persisted evidence must bind to exact admitted source content, and a source change during analysis must fail closed — to #866/Project Persistence through an ordinary released/protected contract. The canonical implementation should hash the admitted immutable publication identity or a descriptor/snapshot with a bounded logical EOF, not re-stat and re-read a mutable pathname. Do not copy mutable #866 internals here. The existing tests for same-size replacement and source mutation are valuable acceptance evidence and should be preserved in the eventual canonical owner. |
|
Single-writer handoff: canonical Resource Admission/cache source-identity owner #866 is now Draft exact |
|
Owner-boundary refresh from canonical #866: keep #1055's unique tempo/privacy semantics, but do not retain a second source-fingerprint/cache-work authority. Resource Admission now carries native verified content identity into Python, scopes persisted cache by source digest, and scopes mutable stem work by source + hashed job identity so equal-metadata replacement audio and concurrent same-source jobs cannot share execution artifacts. Tempo/privacy code should consume that released/protected boundary rather than re-open/stat/hash mutable source paths independently. |
The CR/LF log-forging finding is valid, but this generated implementation remains weaker than the canonical temporal privacy contract: it still emits the selected local-audio path and raw decoder exception text. The same finding is already preserved in #1211 for canonical owner #1055, which requires path-free bounded context plus exception type after the active #866 source lane releases. Restore this duplicate branch to the protected develop tree as an ordinary descendant so it cannot become a second temporal source writer. Preserve the finding through the existing canonical/preservation path rather than merging a weaker repr(path) implementation. No force update, destructive rebase, self-approval, gate weakening, or security-completion claim.
The CR/LF log-forging finding is valid, but this branch's repr(path) implementation still discloses the selected local-audio path and preserves raw decoder exception text. Its focused test only asserts repr(path) on the info call and does not establish the stronger path-free failure contract. Restore the duplicate branch to protected develop as an ordinary descendant. Preserve the valid finding in #1211 for canonical temporal privacy owner #1055, which already specifies attacker-shaped path plus decoder-exception RED and path-free, exception-type-only GREEN after active source owner #866 releases. Also remove the foreign #1176 formatter delta. No force update, destructive rebase, self-approval, gate weakening, or security-completion claim.
This generated lane mixes a valid TemporalAnalyzer CR/LF finding with a harmless numeric-BPM logging style change and a foreign #1176 formatter delta. Its repr(path) mitigation still discloses the local-audio path and logs repr(str(exception)), which is weaker than the canonical #1055 path-free, exception-type-only privacy contract preserved by #1211. Restore all net changes to protected develop as an ordinary descendant. Keep the valid finding in the canonical preservation/owner path instead of maintaining another temporal source writer. No force update, destructive rebase, self-approval, gate weakening, or security-completion claim.
Dependency-order refinement — warning-policy ownerFresh changed-file review shows #1232 legitimately owns analysis warning visibility but also changes this lane's
#1232 must preserve #866 behavior while removing pre-evidence warning suppression; this owner must then preserve #1232's warning policy while keeping path-free/type-only log semantics and adding only the valid bounded wrapped-error evidence from #1237. No source movement is authorized by this comment. |
|
Fresh single-writer review found new overlapping PR #1252. Exact preservation head is now #1252 identifies a valid CWE-117-class finding on protected Route only its valid raw-path/error log-forging regression intent here after #1176 and #866 become protected prerequisites. Canonical adoption should retain this lane's path-free generic log context and exception-type-only log diagnostics, keep the temporary CLI temporal probe absent, and combine the stronger #1237 broken- |
|
2026-09-23 preservation receipts: two weaker TemporalAnalyzer branches moved again after prior owner-boundary repairs. #1229 intervening |
…rvation lane Restore the validated #1229 preservation tree while retaining the generated test commit in ancestry. The intervening test asserted repr(path) disclosure as safe, which conflicts with canonical #1055 path-free logging and would encode the weaker implementation as expected behavior. Signed-off-by: Seongho Bae <me@seonghobae.me>
|
#1229 fresh live continuation Ordinary non-force descendant #1229 When prerequisites permit source adoption here, keep the canonical RED/GREEN boundary unchanged: attacker-shaped CR/LF source identity and decoder-controlled text must be exercised, but success must assert no source-path disclosure and no injected log-record boundary. |
Retain the generated regression commit in ancestry while removing the test that treats repr(path) disclosure as safe. Canonical #1055 keeps local-audio paths out of the log sink; this preservation lane must not encode the weaker disclosure contract as expected behavior. Signed-off-by: Seongho Bae <me@seonghobae.me>
Remove the generated repr(path)/raw-exception logging implementation from the active diff. Canonical #1055 already owns the stronger path-free log sink and a stronger caplog regression that covers decoder failure without disclosing the selected local path. Keep the generated finding in ancestry for provenance until protected succession satisfies PR-0. Signed-off-by: Seongho Bae <me@seonghobae.me>
…-only Remove the duplicate TemporalAnalyzer/sentinel/test delta from the active diff. Canonical #1055 already owns the stronger path-free log sink and a stricter caplog regression covering decoder failure without local-path disclosure; #1237 preserves the separate bounded caller-visible diagnostic finding. Keep this branch history as provenance until verified protected succession satisfies PR-0. Signed-off-by: Seongho Bae <me@seonghobae.me>
1 participant
Canonical temporal privacy / integrity owner
This Draft remains the canonical owner for TemporalAnalyzer log privacy, production temporal orchestration, source-content cache integrity and the bounded local temporal result contract.
develop@314ddeae7b775a4957594b599358c8255617eb2e9d458b5277ba55769f64650f5881a7bb396bd73ddevelop; no source movement is made in this owner while active prerequisite fix(audio): establish canonical local-audio resource policy #866 is still unmerged.Existing product contract retained
Current verification boundary
Historical local verification on this exact tree includes Python 690 passed / 24 skipped with 100% coverage, desktop-core tests, frontend coverage, lint/type/security/bootstrap checks and focused post-reconciliation tests. Ruff format also reproduced the protected-base one-file defect owned by #1176, so those results are not a final merge-GREEN claim and the foreign formatter delta is not copied here.
Every future source movement invalidates predecessor checks/reviews. Final acceptance still requires one unchanged exact head after prerequisites have reached protected ancestry.
Preserved log-injection findings
#1211 remains a weaker preservation branch for the CR/LF log-forging class. Its
repr(path)approach is not accepted here because it still discloses the selected local path. Canonical GREEN remains path-free: generic context + exception type at the log sink, with attacker-shaped path/identifier and decoder text unable to create records or disclose local path information.Fresh review of #1237 found a second, distinct valid delta and a new single-writer conflict.
#1237 conflict
#1237 exact
4ad88b6abb738a8a450d980bb7ebf003bd643721edits this owner'stemporal/analyzer.pyandcli.py. Its current analyzer logs a bounded/escaped path and its CLI reintroduces the temporaryTemporalAnalyzerprobe plus buyer filename logs. Those are weaker than this owner's privacy/orchestration contract and must not be merged or transplanted here.#1237 has therefore been converted to Open / Draft preservation rather than remaining a second canonical temporal source writer.
#1237 valid delta to absorb later
The current canonical analyzer still wraps a decoder failure with:
ValueError(f"Temporal analysis failed: {e}")That calls dependency-controlled
__str__while already handling an exception. #1237 carries a valid RED showing that a hostile/broken__str__can mask the intended failure and that a very large exact-string exception argument can create an unbounded wrapped diagnostic. Its useful repair evidence is the bounded, no-dependency-str/reprmessage boundary and the corresponding tests—not its path/filename logging or CLI probe.After prerequisites are actually protected, this owner must adopt/adapt the valid evidence while retaining the stronger privacy contract:
__str__and oversized wrapped-error regression on this path-free analyzer;__str__/__repr__;__cause__for debugging without trusting its representation;This owner does not cherry-pick #1237 wholesale. It preserves only verified semantic/test evidence that is compatible with the existing privacy and orchestration boundary.
Dependency / source-lane order
#1176 remains the one-file Ruff formatter prerequisite. Active Resource Admission owner #866 is still Open and unmerged; its current contract explicitly blocks competing temporal source mutation until it is released. Therefore this run records the validated #1237 delta here but does not mutate production source, create a parallel temporal successor, force-push, or rebase.
Normal order remains:
#1176 protected integration → #866 protected integration → ordinary/non-force reconciliation of this owner → canonical adoption of the valid #1211/#1237 regressions → fresh exact-head verification/review → normal protected merge.
Security Notes
Merge gate
Keep Draft. Do not merge until #1176 and #866 are protected prerequisites, the valid #1211/#1237 evidence is absorbed without privacy/orchestration regression, one unchanged final head has every applicable repository/central CI/build/security/SAST/SBOM/supply-chain/coverage/review gate terminal-success, all valid findings are resolved, and a qualifying independent non-author last-push approval exists.
No self-approval, admin bypass, force-push, destructive rebase, synthetic status, gate weakening, no-op freshness commit, blind rerun, or predecessor-evidence transfer.