fix(desktop): remove synthetic browser analysis success - #827
fix(desktop): remove synthetic browser analysis success#827seonghobae wants to merge 24 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
|
@coderabbitai review |
|
|
@opencode-agent Please run a fresh review and coverage pass on exact current head |
|
Current-head gate evidence on |
|
Queued @opencode-agent for PR #827 at head |
|
Already queued @opencode-agent on this exact request for PR #827 at head |
Production gap
BandScope had browser-only paths that could manufacture successful product state without the native runtime. This canonical branch first removed synthetic analysis jobs/results from
job_runner.ts, then traced the activeApp.tsxexecution path intoapps/desktop/src/lib/analysis.tsand removed staged fake analysis success and silent browser project-save success from that canonical bridge. Current maintenance also closes two privacy boundaries in the same desktop bridge: dependency-controlled/native YouTube import errors no longer pass directly into buyer-visible UI state, and workspace-fetch failures no longer copy dependency-controlled paths/tokens/tool diagnostics into routine frontend console logs.Protected base:
develop@acdbea6344fe1231c39535b575f4de35e4c607c9.Exact current head:
e169e6b89ea913d5e9ab723695ae7b6f39810876.Test-first repairs
Original runner boundary
261253d5910f87992dd48e7741550531e8dc9f92requires enqueue/retry/cancel to fail closed outside Tauri and prohibits synthetic workspace updates/state.bddd8410108d80c81c043a8a1deb911f7e2815d1removes the mock workspace, fake jobs, timers, demo-song construction, and browser mutation path fromjob_runner.ts.e5e1b7b2046de5e9b55d2c44140589c3c9cdb9d1records the contract in CHANGELOG;9aaff0e2e153d83297d0f667b7ce13e2880dc33fremoves unrelated dependency drift;a9063cb1eca490010e0f6905380304afb908db40alignsCLAUDE.md.Active App analysis bridge correction
App.tsximports its analysis lifecycle from./lib/analysis, notjob_runner.ts.5066f18c94fa1c3103457a3e4c427c70c089c8b8requires browser-onlystartAnalysisJobto return a stableengine_unavailablefailure with no result and requires subsequent status lookup not to find synthetic retained state.380bac0213f80fc098ec9496a673eaa5500cea32removesbrowserJobStore, staged fake progress, and demo-song success from the active bridge. Native Tauri invocation and validated native status parsing remain unchanged.6a976f50f5b7dcc4609a921330f99c3017d493f9removes the obsolete predecessor test that asserted staged fake browser success.Project-save mutation fails closed
Fresh exact-code review found another false-success mutation in the same canonical bridge:
browserFallback("save_project")returnedundefined, sosaveProject()reported success although no project was persisted.038f05c422e944f94633cd4d29b8b6e9ba883e83adds a browser-only regression requiringsaveProject(createDemoRehearsalSong())to reject instead of succeeding. Its workflow cycle was superseded before terminal execution and is not counted as RED CI proof.acd5d3d51e997a67a4e61879502fb849375d8721makes the browsersave_projectcommand throwProject save requires the Tauri runtime.; native Tauri save behavior and request validation are unchanged.d971e35fa2302f561081e754cee448665413a981records that boundary inCHANGELOG.md.YouTube import dependency errors are redacted before UI state
docs/security/app-security.mdrequires URL/metadata trust boundaries to be schema-controlled and sensitive paths, tokens, cookies, full URLs, and native tool diagnostics to stay out of surfaced output. Exact-current source inspection found thatimportYoutubeUrl()caught a Tauri/dependency exception and returned its rawError.message(or raw string) directly as buyer-visibleAnalysisJobError.message.86b2426e2f71be18ce4f245db10366f8d818ccb8addsanalysis.youtube-error-privacy.test.ts: a mocked Tauri rejection contains a YouTube video id,C:\\Users\\Alicepath andtoken=super-secret; the required result contains only one safe next-action message and explicitly excludes all three sensitive markers.c58ff8c08006b426a242d377c8c60184178e9e65introduces the stable buyer-facingYouTube import failed. Try again or choose a local audio file.message and stops propagating dependency-controlled exception text. URL allowlist validation still returns its existing precise local validation message before the bridge; native invocation behavior is otherwise unchanged.52d77828f5f528e19338b45a581b8a595dcd1416records the privacy repair inCHANGELOG.md;66eb2d8c0916ca1eac933c661b9dcecece9471e7restores unrelated historical CHANGELOG integrity without broadening production behavior.Workspace-fetch diagnostics are redacted before console output
Fresh review of the same desktop bridge found that
getWorkspaceState()caught a native/dependency failure and passederror.messageintoconsole.error. That made routine frontend diagnostics another path for local filesystem paths, tokens, or tool stderr to escape even though the function correctly returnednullto the caller.76a9e64efb1c4fa8e9fc144206fffadeab137652adds a focused privacy regression that injects path/token-shaped dependency text and requires the routine console diagnostic to remain stable and payload-free.b722eddb1eec56ed30716e00cfdd6e269464b12aremoves dependency-controlled message data from the log and emits onlyFailed to get workspace state.while preserving the existingnullsafe-failure result.f8112be63fb88d0fab1ba436cab96244be21a809records the boundary inCHANGELOG.md.f8d23d435986d6f409a7c6879c5097a701e2ec12keeps the regression mock hoist-safe without changing production behavior.e169e6b89ea913d5e9ab723695ae7b6f39810876additionally aligns App-level YouTube failure assertions with the same fixed-message privacy contract; it is a test-only successor of the production repair.Exact current scope
Current protected-base diff contains ten bounded files:
CHANGELOG.mdCLAUDE.mdapps/desktop/src/App.test.tsxapps/desktop/src/lib/analysis.browser-fail-closed.test.tsapps/desktop/src/lib/analysis.test.tsapps/desktop/src/lib/analysis.tsapps/desktop/src/lib/analysis.youtube-error-privacy.test.tsapps/desktop/src/lib/job_runner.error-privacy.test.tsapps/desktop/src/lib/job_runner.production.test.tsapps/desktop/src/lib/job_runner.tsThere is no dependency, lockfile, workflow, database, widened network authority, filesystem authority, model, new IPC command, vulnerability suppression, or reviewer-credential change.
Resulting boundary
nullrather than fabricated state;Security Notes
Attack surface
Trust boundary
AnalysisJobErrorrendered by the desktop UIgetWorkspaceState()catch boundary → routine console diagnosticRealistic threat
A dependency or native layer can include full source URLs, local paths/usernames, cookies/tokens, command/tool diagnostics, or other operational details in an exception message. Passing that text through verbatim makes internal diagnostics buyer-visible or log-visible and violates BandScope's redacted-by-default desktop boundary.
Mitigations
Remaining risk
Native/downloader logs remain governed by their owning layers; these repairs prevent raw exception text from crossing the repaired frontend UI/console boundaries but do not claim to sanitize independent native log sinks.
Exact-current-head verification
Fresh repository workflows bind to exact head
e169e6b89ea913d5e9ab723695ae7b6f39810876.Terminal-success repository-owned lanes include:
cirun31974254521secret-scan-gaterun31974254570sbomrun31974254538banditrun31974254518SAST Semgreprun31974254544releaserun31974254532build-baselinerun31974254597Exact-current-head failures were inspected rather than treated generically:
security-auditrun31974254556, job95257263799: checkout/setup/install all succeed; first failing boundary isAudit npm dependencies. Python/Rust audit steps are skipped only because the npm gate fails first.Security Scanrun31974254541:scorecard,dependency-review, andosv-scansucceed.trivy-fscompletes scan/SARIF generation and upload, then fails atPrint Trivy findings that failed the gate.This branch has no JavaScript manifest/root-lock/vulnerability-suppression delta, so those exact failures are the canonical protected-base dependency baseline owned by #783 after current-head inspection; they are not suppressed or duplicated here.
Fresh inline review inspection has zero review threads. There is no qualifying independent non-author current-head approval.
Dependency / merge gate
developand evidence is regenerated.ContextualWisdomLab/.github#1020, whose current head must reach protected centralmainbefore dependent central evidence can be regenerated.Keep Draft and unmerged until the unchanged then-current head has exact desktop tests and 100% owned statement/branch/function/line coverage, lint/typecheck/build, repository and central SAST/security/SBOM/supply-chain/package/release gates, zero actionable threads, qualifying independent non-author last-push approval, and branch protection without bypass. Queued, skipped-required, failed inherited-base, predecessor-head, protected-base, model-only, self/author, or administrative-bypass evidence is not success.