Skip to content

fix(security): redact articulation failure logs - #956

Open
seonghobae wants to merge 5 commits into
developfrom
fix/articulation-log-privacy
Open

fix(security): redact articulation failure logs#956
seonghobae wants to merge 5 commits into
developfrom
fix/articulation-log-privacy

Conversation

@seonghobae

@seonghobae seonghobae commented Aug 20, 2026

Copy link
Copy Markdown
Collaborator

Security/privacy repair

analyze_articulation() is intentionally fail-safe, but protected develop still logs unexpected dependency failures with exc_info=True. That routine path can serialize dependency-controlled exception messages and traceback paths even though the public API returns the neutral articulation result.

Exact current identity

  • Target: protected develop.
  • Independently resolved and incorporated protected tip: 749511c3ad4000090048718f685c6bee6b3d2c25.
  • Exact current head: 198344ed100d7a592b9727546fee55322f79d232.
  • Branch: fix/articulation-log-privacy.
  • Open, non-Draft, unmerged. Do not infer merge readiness from PR state.

The current head is a non-force two-parent merge refresh: predecessor repair head 68f337fe835f928b54e734b4442bee2c64634e01 plus live protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. Its tree is the protected tree with only this PR's three owned files overlaid, so intervening protected product/dependency work is preserved rather than overwritten. The independently resolved merge base is now the live protected tip.

Regression-first repair

RED f13b6e149f274551a78ec1cae81184021533c11e injects /Users/Alice/private-articulation.wav token=super-secret through the existing librosa.onset.onset_strength failure path and requires the established SAFE_DEFAULT, stable operation-level diagnostics, no path/filename/secret payload, and no retained traceback.

Production catches the unexpected exception, preserves the neutral result, and logs only the BandScope-owned operation plus type(error).__name__; raw dependency messages and traceback data are not retained in this routine log path. Module security notes and CHANGELOG.md describe the same boundary.

Pre-merge successor 68f337fe835f928b54e734b4442bee2c64634e01 strengthened the regression so a RuntimeError must produce exactly Articulation analysis failed; returning safe default (RuntimeError) while the injected sensitive payload remains absent and exc_info remains unset. Current merge head 198344ed100d7a592b9727546fee55322f79d232 preserves that test and production repair unchanged while refreshing the protected base.

Scope / ownership

This lane changes only articulation safe-failure logging, its focused regression, and truthful Unreleased documentation. It does not change onset/RMS computation, classification thresholds, articulation mathematics, public result schema, dependencies, lockfiles, models, filesystem/network/IPC authority, workflows, credentials, or vulnerability suppression. Overlap logging remains #873-owned and range-analysis logging remains #951-owned.

#783 JavaScript dependency-security remediation is already protected shipped truth and is preserved by this branch refresh. Any current-head failure must be inspected at its current job/log/checkout boundary before classification.

Verification state

A same-head Devin review on predecessor 68f337fe835f928b54e734b4442bee2c64634e01 reported no issues, but it is predecessor review evidence after the merge refresh and is not counted as approval/current-head proof. The PR currently has no qualifying independent non-author approval.

Fresh repository/central verification for exact current head 198344ed100d7a592b9727546fee55322f79d232 must reach terminal success. No predecessor check, queued/pending/skipped job, protected-base run, or model-only result transfers to this head.

Merge gate

Do not merge or enable auto-merge until one unchanged exact head has every applicable repository and central CI/security/SAST/SBOM/supply-chain/release/coverage/review gate terminal-success, exact required owned coverage/docstring evidence, zero valid unresolved current-head findings, a qualifying independent non-author last-push approval, and ordinary protected-branch acceptance without bypass.

Queued, pending, skipped-required, cancelled, failed, predecessor-head, protected-base, status-only/model-only, self/author, neutral, rate-limited, or administrative-bypass evidence is non-passing.

@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

Next included review available in 21 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6d0d4c34-e4d6-47fc-a822-b66b0d9586cf

📥 Commits

Reviewing files that changed from the base of the PR and between 749511c and 198344e.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • services/analysis-engine/src/bandscope_analysis/roles/articulation.py
  • services/analysis-engine/tests/test_articulation.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@opencode-agent opencode-agent Bot added area: security Security boundary, hardening, or vulnerability prevention priority: medium Normal-priority or P2 work status: draft Draft pull request type: bug Defect or incorrect behavior labels Aug 22, 2026
@seonghobae
seonghobae marked this pull request as ready for review August 26, 2026 13:11
@seonghobae seonghobae removed the status: draft Draft pull request label Aug 26, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Open in Devin Review

Copy link
Copy Markdown
Collaborator Author

@opencode-agent review

Review exact current head 198344ed100d7a592b9727546fee55322f79d232 only against live protected develop@749511c3ad4000090048718f685c6bee6b3d2c25. The branch is the canonical fix/articulation-log-privacy lane; do not mutate source or create another branch/PR as part of review. Repository-local CI/release/security/SBOM/build/SAST and central coverage/opencode checks on this exact head are terminal-success at the latest refetch. Please submit an authenticated formal current-head review if your independent-review policy permits it. Verify the three-file privacy boundary: unexpected articulation dependency failures must return the neutral safe default while logging only the operation plus exception type, with no dependency-controlled message, path, secret-shaped payload, or traceback retention. Treat predecessor-head Devin review and any rate-limited CodeRabbit result as non-passing evidence. If you find a defect, anchor it to this exact head; otherwise APPROVE only if policy permits. Do not weaken gates, touch dependencies/workflows, or reuse predecessor approval evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: security Security boundary, hardening, or vulnerability prevention priority: medium Normal-priority or P2 work type: bug Defect or incorrect behavior

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant