fix(p0-3): redact signed URL + reason in bcut transcribe#429
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 59 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 36cf7b2683
ℹ️ 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".
|
@copilot please address the blocking signals on this PR and push a fix commit. Trigger: blocking-review 💡 Codex ReviewHere are some automated review suggestions for this pull request. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". Follow the PR's |
There was a problem hiding this comment.
Pull request overview
This PR addresses a credential-leak risk in the video-to-text-bcut transcription tool by ensuring signed URLs and exception-derived reasons are redacted in both logs and structured tool outputs.
Changes:
- Redact signed URL inputs via
redact_url()in BCUT transcribe results and log fields. - Redact exception text via
redact()after replacing the raw input URL with its redacted form. - Add regression tests to ensure BCUT logs/results do not contain signed URL query secrets.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
autosearch/skills/tools/video-to-text-bcut/transcribe.py |
Redacts source and sanitizes returned reason/log fields to prevent signed-URL leakage. |
tests/tools/test_url_leak_prevention.py |
Adds tests covering BCUT log and structured output redaction behavior. |
* test(bcut): add failing scaffolds for log + result + output redact (F005 S1) * fix(bcut): redact source URL + reason text in log calls (F005 S2) * fix(bcut): redact source URL in transcribe() return dict (F005 S3) * fix(bcut): guard non-string source before redact + add coerce * fix(bcut): replace LOGGER.exception with redacted LOGGER.warning to avoid traceback leak
…handling (#436) * test(kr36): add failing scaffold for 404 transient_error mapping (F015 S1) * docs(kr36): research candidate endpoints for 36kr search (F015 S2) * fix(kr36): apply S2 result — endpoint update or graceful transient_error (F015 S3) * refactor(kr36): replace monkeypatch identity with explicit feature flag for legacy HTML path * refactor(kr36): remove unreachable raise after _raise_search_endpoint_unavailable * refactor(kr36): annotate _raise_search_endpoint_unavailable as NoReturn * fix(kr36): read article fields from nested templateMaterial payload * fix(kr36): skip empty values when falling back across API keys * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(p0-2): redact local path in cloud transcribe structured output (#428) * test(transcribe-openai): add failing scaffold for local path redact (F004 S3) * feat(redact): add redact_path_for_output helper + apply in video-to-text-openai (F004 S4) * docs(transcribe-openai): clarify SKILL.md output contract for local input (F004 S4) * fix(transcribe-groq): apply redact_path_for_output to local path output (F004 S5) * docs(transcribe-groq): clarify SKILL.md output contract for local input (F004 S5) * test(transcribe): avoid path_redact pytest module collision (F004) * test(redact): use generic fixture path to avoid personal-path gitleaks false positive * Revert "test(redact): use generic fixture path to avoid personal-path gitleaks false positive" This reverts commit 64bfd34. * chore(gitleaks): allowlist /Users/<placeholder>/ in personal-path rule * fix(redact): handle Windows drive-letter paths + schemeless URLs in redact_path_for_output * fix(transcribe-openai): redact extra values in _failure + use redact_path_for_output in logs * fix(transcribe-groq): redact extra values in _failure + use redact_path_for_output in logs * refactor(redact): make redact_path_for_output accept str | None natively (drop type: ignore) * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * fix(p0-1): npm wrapper fake success — ENOENT + PATH persistence + --no-init (#427) * test(npm-wrapper): add failing scaffold for ENOENT detection (F003 S1) * fix(npm-wrapper): exit non-zero on spawn ENOENT with fix hint (F003 S2) * test(install-sh): add failing scaffold for PATH persistence (F003 S3) * fix(install-sh): persist PATH to shell profile only when missing (F003 S4) * fix(npm-wrapper): prepend ~/.local/bin to PATH before post-install spawn (F003 S5) * fix(npm-wrapper): pass --no-init to installer to avoid implicit init (F003 S6) * test(install-script): add e2e smoke for install → spawn happy path (F003 S7) * chore(gitleaks): allowlist install + npm-wrapper paths for .local XDG dirs Tailscale-domain rule's regex was matching POSIX XDG user paths ($HOME/.local/bin) introduced by F003 npm wrapper PATH-persistence work. These are filesystem paths, not mDNS/Tailscale hostnames. Allowlist scoped to scripts/install.sh, npm/bin/*.js, and the related smoke/unit tests; regexes pinned to recognized XDG path components. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(install-sh): prefer sourced bash profile * docs(install-sh): clarify path persistence writes * test(npm-wrapper): skip POSIX wrapper shims on Windows * test(install-script): skip e2e bash stub on Windows * fix(npm-wrapper): handle spawn permission errors * fix(p0-6): redact scenario error + stderr in e2b reports (#432) * test(e2b-report): add failing scaffold for error redaction in summary + results.json (F008 S1) * fix(e2b-report): redact scenario error in summary.md (F008 S2) * fix(e2b-report): redact scenario error in results.json (F008 S3) * fix(e2b-comprehensive): redact scenario error + stderr in transcripts (F008 S4) * chore(gitleaks): widen install/wrapper allowlist to cover new permission test file * fix(install-sh): skip PATH persist for unknown shells (don't write bash syntax to .bashrc) --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style(transcribe): apply ruff format on openai/groq transcribe modules --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * fix(p0-3): redact signed URL + reason in bcut transcribe (#429) * test(bcut): add failing scaffolds for log + result + output redact (F005 S1) * fix(bcut): redact source URL + reason text in log calls (F005 S2) * fix(bcut): redact source URL in transcribe() return dict (F005 S3) * fix(bcut): guard non-string source before redact + add coerce * fix(bcut): replace LOGGER.exception with redacted LOGGER.warning to avoid traceback leak * fix(kr36): align legacy test fixture with new endpoint schema --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Summary
P0 fix for
reports/autosearch-p0-fix-plan.md§3 — bcut (Bilibili Cut) transcribe module logged the raw signed URL in three failure log calls and embedded it in the structured output'ssourceandreasonfields. Signed URLs includeSignature=,Expires=, etc. — credential leakage on transcribe failure.Changes
bcut_audio_extraction_failed,bcut_api_failed,bcut_unexpected_error) now wrap the source URL withredact_url()and the exception text withredact().transcribe()return dict'ssourcefield walksredact_url();reasonfield walksredact().redact()alone does not strip URL query strings embedded inside exception text, so the fix first replaces the input URL withredact_url(source), then appliesredact(...).tests/tools/test_url_leak_prevention.py:test_bcut_log_redacts_source/test_bcut_result_reason_redacts_source/test_bcut_structured_output_redacts_source.Plan
docs/exec-plans/active/autosearch-0426-p0-fix-plan-execution.md§ F005 (S1-S3).Commits
Test plan
pytest tests/tools/test_url_leak_prevention.py— 9 passedpytest tests/unit/ -m "not real_llm and not slow and not network"— 673 passed, 3 skipped🤖 Generated with Claude Code