fix(cli): validate trusted adapter manifests#282
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens coven-cli’s adapter trust model by only treating COVEN_HOME/adapters/<known>.json as “trusted/installed” when it exactly matches the bundled adapter recipe and is a non-symlink regular file, preventing planted manifests from registering attacker-controlled harness commands.
Changes:
- Replace broad
COVEN_HOME/adapters/*.jsonloading with a recipe-name allowlist plus exact-content matching (trusted_adapter_manifest_paths/trusted_adapter_manifest_matches_recipe). - Harden
coven adapter install <recipe>to only treat an adapter as installed when the on-disk manifest matches the bundled recipe, and to replace stale/suspicious manifests. - Add unit + smoke regressions to ensure planted manifests are ignored and installs replace pre-existing untrusted manifests.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| crates/coven-cli/src/harness.rs | Adds trusted-manifest selection and exact-match validation for known bundled adapter recipes. |
| crates/coven-cli/src/main.rs | Updates adapter install flow to use match-based “installed” detection and replace existing manifests. |
| crates/coven-cli/tests/smoke.rs | Adds smoke regression verifying adapter install hermes replaces a planted manifest. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+848
to
+855
| if manifest_path.symlink_metadata().is_ok() { | ||
| std::fs::remove_file(&manifest_path).with_context(|| { | ||
| format!( | ||
| "failed to replace trusted adapter manifest {}", | ||
| manifest_path.display() | ||
| ) | ||
| })?; | ||
| } |
Comment on lines
+355
to
+359
| fn trusted_adapter_manifest_paths(coven_home: &Path) -> Vec<PathBuf> { | ||
| known_adapter_recipe_names() | ||
| .iter() | ||
| .filter_map(|adapter_id| { | ||
| let path = trusted_adapter_manifest_path(coven_home, adapter_id); |
Comment on lines
+367
to
+369
| let manifest = fs::read_to_string(manifest_path)?; | ||
| assert!(manifest.contains("\"executable\": \"hermes\"")); | ||
| assert!(!manifest.contains("\"executable\":\"sh\"")); |
BunsDev
force-pushed
the
codex/fix-implicit-adapter-trust-vulnerability
branch
from
db59690 to
73c7f37
Compare
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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.
Motivation
.jsonunderCOVEN_HOME/adapters, creating an implicit trust path that allowed planted manifests to register harnesses which could execute attacker-controlled commands.Description
adapter_manifest_paths_in_dirscan ofCOVEN_HOME/adapterswithtrusted_adapter_manifest_paths, which only yields manifests for known recipe names that pass exact content matching viatrusted_adapter_manifest_matches_recipe.trusted_adapter_manifest_matches_recipeto check that a manifest is a non-symlink regular file and that its text matches the bundled recipe returned byknown_adapter_manifestbefore treating it as installed.run_adapter_install) to treat a manifest as installed only if it matches the recipe, and to remove symlinked or stale manifests before writing the trusted manifest.COVEN_HOME/adapters/hermes.jsonis ignored, a unit test that a correct bundled Hermes manifest is loaded fromCOVEN_HOME, and a smoke test verifyingcoven adapter install hermesreplaces an existing planted manifest.Testing
cargo fmtto apply formatting without errors.cargo test -p coven-cli adapterwhich executed the adapter unit and smoke tests.Codex Task