Skip to content

Commit 07a7cca

Browse files
committed
feat(plugin): add secure GitHub plugin lifecycle
1 parent f762a21 commit 07a7cca

17 files changed

Lines changed: 1930 additions & 13 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/jcode-base/src/bus.rs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,13 @@ pub struct ManualToolCompleted {
7979
pub duration_ms: u64,
8080
}
8181

82+
#[derive(Clone, Debug)]
83+
pub struct PluginOperationCompleted {
84+
pub session_id: String,
85+
pub output: String,
86+
pub success: bool,
87+
}
88+
8289
/// Type of file operation for swarm awareness
8390
#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
8491
pub enum FileOp {
@@ -397,6 +404,8 @@ pub enum BusEvent {
397404
TodoUpdated(TodoEvent),
398405
SubagentStatus(SubagentStatus),
399406
ManualToolCompleted(ManualToolCompleted),
407+
/// A local `/plugin` operation completed off the UI thread.
408+
PluginOperationCompleted(PluginOperationCompleted),
400409
BatchProgress(BatchProgress),
401410
/// File was touched by an agent (for swarm conflict detection)
402411
FileTouch(FileTouch),

‎crates/jcode-provider-extensions/src/bundle.rs‎

Lines changed: 108 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use super::{ExtensionError, PROVIDER_MANIFEST_FILE, ProviderManifest};
2-
use serde::{Deserialize, Serialize};
2+
use serde::{Deserialize, Deserializer, Serialize, de::Error as _};
33
use std::collections::BTreeSet;
44
use std::fs::{self, File};
55
use std::io::{self, Read};
@@ -46,7 +46,7 @@ pub struct PluginManifest {
4646
pub version: String,
4747
#[serde(default)]
4848
pub description: Option<String>,
49-
#[serde(default)]
49+
#[serde(default, deserialize_with = "deserialize_author")]
5050
pub author: Option<String>,
5151
#[serde(default)]
5252
pub homepage: Option<String>,
@@ -62,6 +62,23 @@ fn default_plugin_manifest_version() -> u32 {
6262
PLUGIN_MANIFEST_VERSION
6363
}
6464

65+
fn deserialize_author<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
66+
where
67+
D: Deserializer<'de>,
68+
{
69+
let value = Option::<serde_json::Value>::deserialize(deserializer)?;
70+
match value {
71+
None => Ok(None),
72+
Some(serde_json::Value::String(author)) => Ok(Some(author)),
73+
Some(serde_json::Value::Object(author)) => author
74+
.get("name")
75+
.and_then(serde_json::Value::as_str)
76+
.map(|name| Some(name.to_string()))
77+
.ok_or_else(|| D::Error::custom("author object must contain a string name")),
78+
Some(_) => Err(D::Error::custom("author must be a string or object")),
79+
}
80+
}
81+
6582
impl PluginManifest {
6683
fn validate(&self) -> Result<(), BundleError> {
6784
if self.manifest_version != PLUGIN_MANIFEST_VERSION {
@@ -184,11 +201,10 @@ impl PluginBundle {
184201
.into_iter()
185202
.filter(|path| path.is_file())
186203
.collect::<Vec<_>>();
187-
let manifest_path = match manifest_candidates.as_slice() {
188-
[] => return Err(BundleError::MissingManifest),
189-
[path] => path.clone(),
190-
paths => return Err(BundleError::AmbiguousManifest(paths.to_vec())),
191-
};
204+
let manifest_path = manifest_candidates
205+
.first()
206+
.cloned()
207+
.ok_or(BundleError::MissingManifest)?;
192208

193209
let manifest_bytes = read_bounded(&manifest_path, MAX_PLUGIN_MANIFEST_BYTES).map_err(
194210
|error| match error {
@@ -210,6 +226,29 @@ impl PluginBundle {
210226
}
211227
})?;
212228
manifest.validate()?;
229+
for candidate in manifest_candidates.iter().skip(1) {
230+
let candidate_bytes = read_bounded(candidate, MAX_PLUGIN_MANIFEST_BYTES).map_err(
231+
|error| match error {
232+
BoundedReadError::TooLarge => BundleError::ManifestTooLarge {
233+
path: candidate.clone(),
234+
limit: MAX_PLUGIN_MANIFEST_BYTES,
235+
},
236+
BoundedReadError::Io(source) => BundleError::Read {
237+
path: candidate.clone(),
238+
source,
239+
},
240+
},
241+
)?;
242+
let candidate_manifest: PluginManifest = serde_json::from_slice(&candidate_bytes)
243+
.map_err(|source| BundleError::ParseManifest {
244+
path: candidate.clone(),
245+
source,
246+
})?;
247+
candidate_manifest.validate()?;
248+
if !compatible_manifests(&manifest, &candidate_manifest) {
249+
return Err(BundleError::AmbiguousManifest(manifest_candidates));
250+
}
251+
}
213252

214253
let provider_path = root.join(PROVIDER_MANIFEST_FILE);
215254
let provider_manifest = if provider_path.is_file() {
@@ -236,6 +275,22 @@ impl PluginBundle {
236275
}
237276
}
238277

278+
fn compatible_manifests(left: &PluginManifest, right: &PluginManifest) -> bool {
279+
left.manifest_version == right.manifest_version
280+
&& left.name == right.name
281+
&& left.version == right.version
282+
&& optional_field_matches(&left.homepage, &right.homepage)
283+
&& optional_field_matches(&left.repository, &right.repository)
284+
&& optional_field_matches(&left.license, &right.license)
285+
}
286+
287+
fn optional_field_matches(left: &Option<String>, right: &Option<String>) -> bool {
288+
match (left, right) {
289+
(Some(left), Some(right)) => left == right,
290+
_ => true,
291+
}
292+
}
293+
239294
fn discover_skills(root: &Path) -> Result<Vec<SkillMetadata>, BundleError> {
240295
let skills_root = root.join("skills");
241296
if !skills_root.is_dir() {
@@ -474,4 +529,50 @@ mod tests {
474529
Err(BundleError::AmbiguousManifest(_))
475530
));
476531
}
532+
533+
#[test]
534+
fn accepts_equivalent_claude_and_codex_manifests() {
535+
let dir = tempdir().unwrap();
536+
fs::create_dir_all(dir.path().join(".claude-plugin")).unwrap();
537+
fs::create_dir_all(dir.path().join(".codex-plugin")).unwrap();
538+
let manifest = r#"{"name":"shared-plugin","version":"1.0.0"}"#;
539+
fs::write(dir.path().join(".claude-plugin/plugin.json"), manifest).unwrap();
540+
fs::write(dir.path().join(".codex-plugin/plugin.json"), manifest).unwrap();
541+
542+
let bundle = PluginBundle::load(dir.path()).unwrap();
543+
assert_eq!(bundle.manifest.name, "shared-plugin");
544+
}
545+
546+
#[test]
547+
fn accepts_platform_specific_metadata_for_same_plugin_identity() {
548+
let dir = tempdir().unwrap();
549+
fs::create_dir_all(dir.path().join(".claude-plugin")).unwrap();
550+
fs::create_dir_all(dir.path().join(".codex-plugin")).unwrap();
551+
fs::write(
552+
dir.path().join(".codex-plugin/plugin.json"),
553+
r#"{"name":"shared-plugin","version":"1.0.0","description":"Codex description","keywords":["codex"],"repository":"https://github.com/example/shared-plugin"}"#,
554+
)
555+
.unwrap();
556+
fs::write(
557+
dir.path().join(".claude-plugin/plugin.json"),
558+
r#"{"name":"shared-plugin","version":"1.0.0","description":"Claude description","keywords":["claude"],"repository":"https://github.com/example/shared-plugin"}"#,
559+
)
560+
.unwrap();
561+
562+
let bundle = PluginBundle::load(dir.path()).unwrap();
563+
assert_eq!(bundle.manifest.name, "shared-plugin");
564+
}
565+
566+
#[test]
567+
fn accepts_structured_author_metadata() {
568+
let dir = tempdir().unwrap();
569+
fs::write(
570+
dir.path().join("plugin.json"),
571+
r#"{"name":"structured-author","version":"1.0.0","author":{"name":"Jesse Vincent","email":"jesse@example.com"}}"#,
572+
)
573+
.unwrap();
574+
575+
let bundle = PluginBundle::load(dir.path()).unwrap();
576+
assert_eq!(bundle.manifest.author.as_deref(), Some("Jesse Vincent"));
577+
}
477578
}

‎crates/jcode-provider-extensions/src/lib.rs‎

Lines changed: 80 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,17 @@
55
//! deadlines, cancellation, and frame limits outside the persistence layer.
66
77
mod bundle;
8+
mod remote;
89
mod runtime;
910

1011
pub use bundle::{
1112
BundleComponents, BundleError, PLUGIN_MANIFEST_VERSION, PluginBundle, PluginManifest,
1213
SkillMetadata,
1314
};
15+
pub use remote::{
16+
InstalledPlugin, PluginInstallMetadata, PluginSource, PluginStore, PluginStoreSnapshot,
17+
RemotePluginError,
18+
};
1419
pub use runtime::{
1520
EmbeddedExtension, EmbeddedExtensionManifest, ExtensionBackend, ExtensionEvent,
1621
ExtensionInvocation, ExtensionRequest, ExtensionRuntimeRegistry,
@@ -86,6 +91,8 @@ pub enum ExtensionError {
8691
RequestFailed(String),
8792
#[error("extension cancellation is not supported by '{0}'")]
8893
CancellationUnsupported(String),
94+
#[error("plugin operation failed: {0}")]
95+
Plugin(#[from] RemotePluginError),
8996
}
9097

9198
#[derive(Debug, Clone, Default)]
@@ -498,6 +505,41 @@ impl ProviderRegistry {
498505
Ok(())
499506
}
500507

508+
/// Register a provider from an installed plugin, or update that plugin's
509+
/// existing provider while preserving its enabled and trust state.
510+
///
511+
/// A provider ID may only be replaced when the previous source is inside
512+
/// the same plugin root. This prevents a plugin install from silently
513+
/// taking over an unrelated user-managed provider with the same ID.
514+
pub fn register_or_update_plugin(
515+
&mut self,
516+
manifest: ProviderManifest,
517+
source: PathBuf,
518+
plugin_root: &Path,
519+
trusted: bool,
520+
) -> Result<(), ExtensionError> {
521+
manifest.validate()?;
522+
if !source.starts_with(plugin_root) {
523+
return Err(ExtensionError::InvalidManifest(
524+
"plugin provider source must remain inside its plugin root".to_string(),
525+
));
526+
}
527+
if let Some(existing) = self.providers.get_mut(&manifest.id) {
528+
if !existing
529+
.source
530+
.as_deref()
531+
.is_some_and(|path| path.starts_with(plugin_root))
532+
{
533+
return Err(ExtensionError::DuplicateProvider(manifest.id));
534+
}
535+
existing.manifest = manifest;
536+
existing.source = Some(source);
537+
existing.trusted |= trusted;
538+
return Ok(());
539+
}
540+
self.register(manifest, Some(source), trusted)
541+
}
542+
501543
pub fn remove(&mut self, id: &str) -> Result<ProviderRecord, ExtensionError> {
502544
self.providers
503545
.remove(id)
@@ -513,6 +555,15 @@ impl ProviderRegistry {
513555
Ok(())
514556
}
515557

558+
pub fn set_trusted(&mut self, id: &str, trusted: bool) -> Result<(), ExtensionError> {
559+
let record = self
560+
.providers
561+
.get_mut(id)
562+
.ok_or_else(|| ExtensionError::MissingProvider(id.to_string()))?;
563+
record.trusted = trusted;
564+
Ok(())
565+
}
566+
516567
pub fn save(&self) -> Result<(), ExtensionError> {
517568
if let Some(parent) = self.path.parent() {
518569
fs::create_dir_all(parent).map_err(|source| ExtensionError::Write {
@@ -628,6 +679,35 @@ permissions = ["network"]
628679
assert_eq!(loaded.list().count(), 0);
629680
}
630681

682+
#[test]
683+
fn plugin_provider_update_preserves_state_and_rejects_foreign_owner() {
684+
let dir = tempdir().unwrap();
685+
let plugin_root = dir.path().join("plugin");
686+
let old_source = plugin_root.join("versions/1.0.0+old");
687+
let new_source = plugin_root.join("versions/1.1.0+new");
688+
let foreign_source = dir.path().join("other/versions/1.1.0+new");
689+
let mut registry = ProviderRegistry::open(dir.path().join("providers.json")).unwrap();
690+
registry
691+
.register(manifest("fixture-provider"), Some(old_source), true)
692+
.unwrap();
693+
registry.set_enabled("fixture-provider", false).unwrap();
694+
695+
let mut updated = manifest("fixture-provider");
696+
updated.version = "1.1.0".to_string();
697+
registry
698+
.register_or_update_plugin(updated.clone(), new_source, &plugin_root, false)
699+
.unwrap();
700+
let record = registry.get("fixture-provider").unwrap();
701+
assert_eq!(record.manifest.version, "1.1.0");
702+
assert!(!record.enabled);
703+
assert!(record.trusted);
704+
705+
assert!(matches!(
706+
registry.register_or_update_plugin(updated, foreign_source, &plugin_root, false,),
707+
Err(ExtensionError::InvalidManifest(_))
708+
));
709+
}
710+
631711
#[test]
632712
fn missing_registry_starts_empty() {
633713
let dir = tempdir().unwrap();

0 commit comments

Comments
 (0)