feat(plugins): secure yt-dlp and centralize plugin CI - #168
Merged
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
📝 WalkthroughWalkthroughThe change adds a constrained yt-dlp broker, checksum-backed official-plugin provenance, provenance-aware store installation, broker-based media paths, and reusable CI/release workflows for manifest, ABI, artifact, release, and registry verification. ChangesPlugin security and release verification
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Store
participant StoreInstall
participant PluginLoader
participant YtDlpBroker
participant yt-dlp
Store->>StoreInstall: download authoritative plugin
StoreInstall->>PluginLoader: load official directory with provenance
PluginLoader->>PluginLoader: verify manifest and WASM checksums
PluginLoader->>YtDlpBroker: build granted host functions
YtDlpBroker->>yt-dlp: execute validated request
yt-dlp-->>YtDlpBroker: return bounded output
sequenceDiagram
participant TagWorkflow
participant PluginCI
participant Artifact
participant GitHubRelease
participant Registry
TagWorkflow->>PluginCI: invoke reusable verification workflow
PluginCI->>Artifact: upload canonical dist bundle
TagWorkflow->>Artifact: download verified bundle
TagWorkflow->>GitHubRelease: create or validate immutable assets
TagWorkflow->>Registry: compare version and checksums
Registry-->>TagWorkflow: return consistency result
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
github-actions
Bot
added
documentation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
Merging this PR will not alter performance
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ❌ | normalize_max_concurrent |
120.8 ns | 179.2 ns | -32.56% |
| ❌ | from_status_code_500 |
94.7 ns | 123.9 ns | -23.54% |
| ⚡ | from_status_code_200 |
123.3 ns | 94.2 ns | +30.97% |
| ⚡ | from_status_code_404 |
153.1 ns | 123.9 ns | +23.54% |
| ⚡ | split |
471.9 ns | 413.6 ns | +14.1% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing feat/lot-1 (c2d55fb) with main (60bb867)
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 25 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 26 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 5 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/plugin-ci.yml:
- Around line 146-172: Update the “Build verified release bundle” step to remove
any existing dist directory contents before recreating it, ensuring stale files,
directories, and symlinks cannot survive. Preserve the existing canonical asset
copying, checksum generation, and asset-set validation.
- Around line 25-26: Update the actions/checkout step in the plugin CI workflow
to set persist-credentials to false, preventing the checkout token from
remaining available to subsequent Cargo build scripts and tests.
- Around line 106-141: Update the “Verify WASM artifact and ABI exports” step to
inspect the module’s actual WASM export table rather than llvm-nm’s
defined-symbol list. Preserve the package-specific required export sets and fail
with the existing missing-export behavior when any required name is absent from
the exported functions.
In @.github/workflows/plugin-release.yml:
- Around line 162-177: Update the registry lookup around entry selection to
collect all entries whose name matches manifest["name"] and require exactly one
match, rejecting zero or duplicate entries before validating metadata and
checksums. Use the sole matching entry for the existing version and checksum
comparisons, preserving the current error behavior for a missing plugin.
In `@src-tauri/src/adapters/driven/plugin/extism_loader.rs`:
- Around line 85-90: Update the path validation after resolve_path in the plugin
loading flow to use filesystem-aware canonical paths or filesystem identity
comparison for both plugins_dir and provenance_path, preventing case-only
aliases from bypassing containment checks. Preserve the existing
DomainError::ValidationError response for provenance inside the plugin
directory, and add a regression test covering case-insensitive path casing.
In `@src-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/platform.rs`:
- Around line 12-18: Update the test setup around find_approved_binary to name
the symlink-escape candidate yt-dlp and place it inside a separate subdirectory
under the approved directory. Ensure the symlink still targets unsafe_binary
outside the approved tree, while valid remains the direct approved executable,
so filename validation passes and target/ancestor validation is exercised.
In `@src-tauri/src/adapters/driven/plugin/ytdlp_broker/validation.rs`:
- Around line 136-152: Update ensure_private_root and the
builder.create(¤t) initialization path to treat an AlreadyExists error as
successful concurrent creation, then validate the resulting directory with
validate_private_root. Continue propagating other creation errors unchanged and
preserve the existing private-permission checks.
- Around line 14-24: Update the URL validation around parsed.scheme() to reject
every URL whose scheme is not HTTPS before the provider-specific Generic
canonicality checks. Preserve the existing Generic restrictions for credentials
and non-443 ports, while allowing only HTTPS URLs for all providers.
In `@src-tauri/src/adapters/driving/tauri_ipc.rs`:
- Around line 1808-1810: Update the destination handling around the
produced_canonical rename/copy flow to reserve dest_path atomically without
replacing an existing file. Use an atomic no-replace rename or a
create_new(true) reservation-and-retry loop, and ensure concurrent downloads
select a new path when reservation fails.
In `@src-tauri/src/application/commands/store_install.rs`:
- Around line 87-93: Update the installation flow around the
`tokio::task::spawn_blocking` result to capture both join and loader errors
instead of propagating immediately. Invoke the existing staging-directory
cleanup before returning the captured error, then propagate the result while
preserving the current `AppError` mappings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 760e03d6-f51b-4c4c-aec8-71ae8c390eae
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/workflows/plugin-ci.yml.github/workflows/plugin-release.ymlCHANGELOG.mdREADME.mddeny.tomlsrc-tauri/Cargo.tomlsrc-tauri/src/adapters/driven/plugin/capabilities.rssrc-tauri/src/adapters/driven/plugin/extism_loader.rssrc-tauri/src/adapters/driven/plugin/host_functions.rssrc-tauri/src/adapters/driven/plugin/manifest.rssrc-tauri/src/adapters/driven/plugin/mod.rssrc-tauri/src/adapters/driven/plugin/provenance.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/legacy.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/legacy_download.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/output.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/platform.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/process.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/request.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/selectors.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/audio.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/legacy.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/mod.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/platform.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/process.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/tests/request.rssrc-tauri/src/adapters/driven/plugin/ytdlp_broker/validation.rssrc-tauri/src/adapters/driving/tauri_ipc.rssrc-tauri/src/application/commands/store_install.rssrc-tauri/src/domain/ports/driven/plugin_loader.rssrc-tauri/src/domain/ports/driven/plugin_store_client.rs
💤 Files with no reviewable changes (1)
- deny.toml
12 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Validation
Linear: MAT-129, MAT-131
Summary by cubic
Secures the yt-dlp integration and centralizes plugin CI. Also closes broker validation races and atomically reserves adaptive download outputs to prevent overwrites.
New Features
run_subprocessshim andrun_ytdlp; persists, revalidates, reconciles, and completes Store provenance outside user‑writable paths; grants only to Store‑installed official plugins matching freshly fetched registry SHA256s; app uses the broker for generic metadata and managed fallback downloads.Bug Fixes
reserve_unique_destination).extism1.30.0and addedcommand-group5.0.1for safe process‑group management.Written for commit c2d55fb. Summary will update on new commits.
Summary by CodeRabbit