fix: harden reviewed trust boundaries

changelog.d/review-hardening.fixed.md

@@ -0,0 +1 @@
+Hardened project plugin permissions and OAuth endpoint validation, bounded compaction overflow retries, and retained failed session snapshots for retry.

n00n-agent/src/agent/compaction.rs

@@ -122,8 +122,12 @@ pub(super) async fn compact_history(
         .await
         {
             Ok(response) => break response,
-            Err(e) if e.is_context_overflow() => {
+            Err(error) if error.is_context_overflow() => {
+                let previous_len = compaction_history.len();
                 truncate_oldest_round(&mut compaction_history);
+                if compaction_history.len() == previous_len {
+                    return Err(error);
+                }
             }
             Err(e) => return Err(e),
         }
@@ -324,11 +328,15 @@ mod tests {
     use crate::AgentConfig;
 
     struct MockProvider {
-        responses: Mutex<Vec<StreamResponse>>,
+        responses: Mutex<Vec<Result<StreamResponse, AgentError>>>,
     }
 
     impl MockProvider {
         fn new(responses: Vec<StreamResponse>) -> Self {
+            Self::with_results(responses.into_iter().map(Ok).collect())
+        }
+
+        fn with_results(responses: Vec<Result<StreamResponse, AgentError>>) -> Self {
             Self {
                 responses: Mutex::new(responses),
             }
@@ -349,7 +357,7 @@ mod tests {
             Box::pin(async {
                 let mut responses = self.responses.lock().unwrap();
                 assert!(!responses.is_empty(), "MockProvider: no more responses");
-                Ok(responses.remove(0))
+                responses.remove(0)
             })
         }
 
@@ -454,6 +462,36 @@ mod tests {
         });
     }
 
+    #[test]
+    fn compact_history_returns_overflow_when_nothing_can_be_truncated() {
+        smol::block_on(async {
+            let provider = MockProvider::with_results(vec![Err(AgentError::Api {
+                status: 413,
+                message: "context overflow".into(),
+            })]);
+            let model = default_model();
+            let (raw_tx, _rx) = flume::unbounded();
+            let mut history = History::new(Vec::new());
+
+            let error = compact_history(
+                &provider,
+                &model,
+                &mut history,
+                &EventSender::new(raw_tx, 0),
+                &CancelToken::none(),
+                CompactionTrigger::Manual,
+                None,
+                std::path::Path::new("/tmp"),
+                None,
+            )
+            .await
+            .unwrap_err();
+
+            assert!(error.is_context_overflow());
+            assert!(history.as_slice().is_empty());
+        });
+    }
+
     #[test_case(159_999, 0,       0,       0,      200_000, false ; "below_threshold")]
     #[test_case(160_000, 0,       0,       0,      200_000, true  ; "at_threshold")]
     #[test_case(100,     0,       0,       0,      100,     true  ; "tiny_context_window")]

n00n-agent/src/mcp/oauth/discovery.rs

@@ -1,7 +1,8 @@
 use std::io::Read;
+use std::net::IpAddr;
 
 use isahc::HttpClient;
-use isahc::http::Request;
+use isahc::http::{Request, Uri};
 use serde::Deserialize;
 
 use super::OAuthError;
@@ -96,16 +97,26 @@ pub(super) fn server_origin(server_url: &str) -> String {
 }
 
 fn validate_endpoint_url(url: &str) -> Result<(), OAuthError> {
-    if url.starts_with("https://")
-        || url.starts_with("http://127.0.0.1")
-        || url.starts_with("http://localhost")
-    {
-        Ok(())
-    } else {
-        Err(OAuthError::Other(format!(
-            "endpoint URL must use HTTPS: {url}"
-        )))
+    let uri = url
+        .parse::<Uri>()
+        .map_err(|error| OAuthError::Other(format!("invalid endpoint URL '{url}': {error}")))?;
+    let scheme = uri.scheme_str();
+    let host = uri.host();
+    let secure = scheme == Some("https") && host.is_some();
+    let loopback = scheme == Some("http")
+        && host.is_some_and(|host| {
+            let host = host.trim_start_matches('[').trim_end_matches(']');
+            host.eq_ignore_ascii_case("localhost")
+                || host
+                    .parse::<IpAddr>()
+                    .is_ok_and(|address| address.is_loopback())
+        });
+    if secure || loopback {
+        return Ok(());
     }
+    Err(OAuthError::Other(format!(
+        "endpoint URL must use HTTPS or an exact loopback host: {url}"
+    )))
 }
 
 /// Validates that all required endpoints in auth server metadata use HTTPS.
@@ -278,10 +289,15 @@ mod tests {
         assert_eq!(well_known_url(base, name), expected);
     }
 
-    #[test_case("https://example.com/token",   true  ; "https_valid")]
-    #[test_case("http://127.0.0.1:8080/token", true  ; "localhost_valid")]
-    #[test_case("http://example.com/token",    false ; "http_rejected")]
-    #[test_case("ftp://example.com/token",     false ; "ftp_rejected")]
+    #[test_case("https://example.com/token",                  true  ; "https_valid")]
+    #[test_case("http://127.0.0.1:8080/token",                true  ; "ipv4_loopback_valid")]
+    #[test_case("http://[::1]:8080/token",                    true  ; "ipv6_loopback_valid")]
+    #[test_case("http://localhost:8080/token",                true  ; "localhost_valid")]
+    #[test_case("http://localhost.attacker.invalid/token",    false ; "localhost_prefix_rejected")]
+    #[test_case("http://127.0.0.1.attacker.invalid/token",    false ; "ipv4_prefix_rejected")]
+    #[test_case("http://example.com/token",                   false ; "http_rejected")]
+    #[test_case("ftp://example.com/token",                    false ; "ftp_rejected")]
+    #[test_case("not a url",                                  false ; "malformed_rejected")]
     fn endpoint_url_validation(url: &str, should_pass: bool) {
         assert_eq!(validate_endpoint_url(url).is_ok(), should_pass);
     }

n00n-lua/src/plugin_permissions.rs

@@ -2,6 +2,7 @@ use std::fmt;
 use std::path::Path;
 
 use mlua::{Error as LuaError, Function, IntoLuaMulti, Lua, Result as LuaResult};
+use toml::Value;
 use tracing::warn;
 
 const MANIFEST_FILE: &str = "plugin.toml";
@@ -65,14 +66,13 @@ impl PluginPermissions {
     }
 
     pub fn from_manifest(manifest: &toml::Value) -> Self {
-        const DEFAULT_PERMISSION: bool = true;
         let perms = manifest.get("permissions");
-        let mut allowed = [true; 5];
+        let mut allowed = [false; 5];
         for perm in Permission::ALL {
             allowed[perm as usize] = perms
                 .and_then(|p| p.get(perm.manifest_key()))
-                .and_then(toml::Value::as_bool)
-                .unwrap_or_else(|| DEFAULT_PERMISSION);
+                .and_then(Value::as_bool)
+                == Some(true);
         }
         Self { allowed }
     }
@@ -182,25 +182,25 @@ mod tests {
         let val: toml::Value = toml::from_str(
             r"
             [permissions]
-            fs_read = false
+            fs_read = true
             net = false
             ",
         )
         .unwrap();
         let p = PluginPermissions::from_manifest(&val);
-        assert!(!p.is_allowed(Permission::FsRead));
-        assert!(p.is_allowed(Permission::FsWrite));
+        assert!(p.is_allowed(Permission::FsRead));
+        assert!(!p.is_allowed(Permission::FsWrite));
         assert!(!p.is_allowed(Permission::Net));
-        assert!(p.is_allowed(Permission::Run));
-        assert!(p.is_allowed(Permission::Env));
+        assert!(!p.is_allowed(Permission::Run));
+        assert!(!p.is_allowed(Permission::Env));
     }
 
     #[test]
     fn from_manifest_missing_section() {
         let val: toml::Value = toml::from_str("[package]\nname = \"test\"").unwrap();
         let p = PluginPermissions::from_manifest(&val);
         for perm in Permission::ALL {
-            assert!(p.is_allowed(perm), "{perm} should default to allowed");
+            assert!(!p.is_allowed(perm), "{perm} should default to denied");
         }
     }