fix(agent): refuse invisible format characters in metadata

crates/agent-identity-conformance/COVERAGE.md

@@ -14,6 +14,8 @@ The `p_*` tests check an authority and the `a_*` tests the agent; the tests name
 | §2 `u` read as text: `user@`, empty user name, `@` in path, non-ASCII host           | `a_pending_file_deleted_on_permanent_error`                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                             |
 | §2.1 precedence of `token_url_mismatch`                                              | `p_enroll_csr_url_refusal_precedes_key_lookup_and_token_state` (over the key lookup, a revoked token and an exhausted one), `p_enroll_csr_url_refusal_precedes_token_expiry`; `p_enroll_csr_must_name_token_url` (`invalid_request` for a CSR that also has a bad self-signature)                                                                                                                                                                                                                                                                                                                                                                                                                                                                       |
 | §3 metadata limits                                                                   | `p_metadata_limits`, `p_channel_hello_invalid_metadata_rejected`                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                        |
+| §3 invisible format characters                                                       | `p_metadata_invisible_format_characters` (each end of every listed range refused in the hostname, and U+202E in an unknown key, at enrollment without consuming a token use; each code point just outside a range and the unlisted `Cf` code points U+0600, U+110BD and U+1D173 accepted and stored as sent), `p_channel_hello_invalid_metadata_rejected` (U+E007F in `Hello`); the harness test `invisible_format_character_lists_match_the_contract` pins both lists                                                                                                                                                                                                                                                                                  |
+| §3.1 invisible format characters removed, empty keys `unknown` or omitted            | `a_metadata_invisible_format_characters_removed`, through the metadata override, which also checks that the agent keeps the code points beside the list and omits an optional key (`domain`) left empty; the metadata check of every agent test refuses a listed code point in any value the agent sent, which the harness test `agent_metadata_check_refuses_listed_format_characters_only` pins                                                                                                                                                                                                                                                                                                                                                       |
 | §4 certificates, §5.1 trust anchors                                                  | `p_trust_anchor_lists_roots`, `p_renew_rejects_sha384_csr`, `p_compressed_point_csr_rejected`, `p_csr_bad_self_signature_rejected`, the `csr` vectors; `p_failed_enrollment_consumes_nothing` sends the `bad_self_signature` and `ecdsa_with_sha384` vectors at enrollment, where their enrollment URL (`https://example.test/dvls`) is not the target's `u`, and §2.1 orders `invalid_request` before the `token_url_mismatch` it would draw, so it isolates their rules there                                                                                                                                                                                                                                                                         |
 | §4 chain signature algorithms, CA basicConstraints and keyCertSign                   | the chain check of `p_trust_anchor_lists_roots`, `p_reset_rebases_root_validity`, `p_renew_happy_path`, `p_renew_within_grace`, `p_rotation_publishes_both_roots_and_issues_from_new`, `p_rotation_grace_renewal_after_early_completion` and `p_rotation_old_root_cert_renewable_after_deadline`, which verifies the root's self-signature too; the harness tests `chain_verifies_p384_intermediate_and_refuses_other_algorithms` and `chain_requires_ca_extensions_on_every_issuer` pin that it accepts a P-384 issuer and refuses another curve and each missing CA extension                                                                                                                                                                         |
 | §4 leaf extensions                                                                   | the leaf profile check of `p_trust_anchor_lists_roots` (enroll), `p_renew_happy_path` and `p_renew_within_grace` (renew); the harness test `leaf_extensions_outside_the_profile_are_named` pins that the check refuses basicConstraints and names it, and refuses a leaf without the enrollment URL extension, with two, with a critical one, with one holding a `UTF8String` or naming another URL                                                                                                                                                                                                                                                                                                                                                     |
@@ -47,6 +49,9 @@ The `p_*` tests check an authority and the `a_*` tests the agent; the tests name
 - Verified by review only: the randomness of secrets, nonces and challenges, and the stability of `authority_id` across restores.
 - Known gaps, with no test yet:
   - the §1 conventions beyond the error body, timestamps, repeated members, nesting depth, invalid Unicode and canonical `base64`;
+  - a renew or check-in body whose metadata holds a control character or an invisible format character (§3), which `p_metadata_limits` and `p_metadata_invisible_format_characters` send only at enrollment and `p_channel_hello_invalid_metadata_rejected` only in `Hello`;
+  - a code point inside a listed range of §3 other than its ends;
+  - the agent's removal of C0, DEL and C1 control characters (§3.1), and its removal of invisible format characters at renew, check-in and `Hello`, because no test sets such a value in the metadata override before those sends;
   - an `agent_channel_url` holding an `@`, which the agent treats as absent (§5.2, §7.2), because no mock fault delivers such a URL;
   - the shape of the agent's own CSR (§4: one attribute, an `extensionRequest` holding only the enrollment URL extension) and the URL its renewal CSR names, because the mock reports no CSR it received; the mock accepting the agent's enrollments shows only that their CSRs name the token's `u` where the mock reads it;
   - the agent's check of an enrollment that returns a known authority ID from another base URL ("Pending-enrollment files" in `crates/agent-identity/INTENT.md`), such as a chain through a P-384 CA accepted and an RSA-signed chain refused with `authority_conflict`, because a mock serves one base URL and issues only from its own root.

crates/agent-identity-conformance/src/agent.rs

@@ -23,8 +23,9 @@ use tokio::process::{Child, Command};
 use x509_cert::Certificate;
 
 use crate::client::{
-    Target, Token, count, decoded_certificate, expect_error, expect_status, field, token_bag, token_base_url,
-    with_bag_url, with_upper_case_scheme,
+    Target, Token, count, decoded_certificate, expect_error, expect_status, field,
+    invisible_format_character_range_ends, is_invisible_format_character, token_bag, token_base_url,
+    unlisted_format_character_neighbors, with_bag_url, with_upper_case_scheme,
 };
 use crate::dvls::{CertificateStatus, IdentityQuery, IdentityRecord, IdentityStatus, RotationDeadline};
 use crate::{Context, KeyBackend};
@@ -234,6 +235,10 @@ fn validate_agent_metadata(metadata: &Value, agent_version: Option<&str>) -> any
                 .all(|character| character >= '\u{20}' && !('\u{7f}'..='\u{9f}').contains(&character)),
             "agent metadata {key} contains a C0/C1 control character"
         );
+        ensure!(
+            !value.chars().any(is_invisible_format_character),
+            "agent metadata {key} contains an invisible format character"
+        );
         total_bytes += key.len() + value.len();
     }
     ensure!(total_bytes <= 16 * 1024, "agent metadata exceeds 16 KiB");
@@ -843,10 +848,18 @@ impl AgentCase {
     }
 
     fn set_metadata_hostname(&mut self, hostname: &str) -> anyhow::Result<()> {
+        self.set_metadata_override(&json!({ "hostname": hostname }))?;
+        self.metadata_hostname = hostname.to_owned();
+        Ok(())
+    }
+
+    /// Replaces the metadata override with `values`, which the agent reads before its next send.
+    ///
+    /// It leaves `metadata_hostname`, the hostname that the metadata checks expect, to the caller.
+    fn set_metadata_override(&self, values: &Value) -> anyhow::Result<()> {
         let replacement = self.path().join("identity-metadata-override-next.json");
-        std::fs::write(&replacement, serde_json::to_vec(&json!({ "hostname": hostname }))?)?;
+        std::fs::write(&replacement, serde_json::to_vec(values)?)?;
         std::fs::rename(&replacement, self.metadata_override_path())?;
-        self.metadata_hostname = hostname.to_owned();
         Ok(())
     }
 
@@ -4473,6 +4486,51 @@ pub(crate) async fn a_channel_connected_and_metadata(ctx: Context) -> anyhow::Re
     .await
 }
 
+/// CONTRACT.agent-identity.md §3.1: the agent "removes [...] the invisible format characters of §3" from each value it sends, and "A required key that ends up empty gets the value `unknown`".
+///
+/// The metadata override replaces collected values before each send (the agent's intent, "Configuration"), so the values this test sets go through that removal.
+/// The hostname holds every end of a listed range before, between and after code points that the list leaves out, which the agent keeps.
+/// The OS name, a required key, and the domain, an optional one, hold only listed code points, so the agent sends `unknown` for the first and omits the second.
+pub(crate) async fn a_metadata_invisible_format_characters_removed(ctx: Context) -> anyhow::Result<()> {
+    // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-metadata
+    // oracle: intent devolutions-agent/src/identity/INTENT.md §Configuration
+    with_agent(ctx, None, true, async move |case| {
+        let target = case.ctx.target.clone();
+        let listed = invisible_format_character_range_ends();
+        let only_listed = listed.iter().collect::<String>();
+        let mut hostname = format!("{only_listed}host");
+        let mut expected = "host".to_owned();
+        for (separator, kept) in listed.iter().cycle().zip(unlisted_format_character_neighbors()) {
+            hostname.push(*separator);
+            hostname.push(kept);
+            expected.push(kept);
+        }
+        hostname.push_str("name");
+        hostname.push_str(&only_listed);
+        expected.push_str("name");
+        case.set_metadata_override(&json!({ "hostname": hostname, "os_name": only_listed, "domain": only_listed }))?;
+        let (id, _) = case.enroll_new(&target, 1).await?;
+        let record = case.identity_record(&target, &id).await?;
+        ensure!(
+            record.metadata_value("hostname") == Some(&*expected),
+            "agent sent hostname {:?} instead of {expected:?}",
+            record.metadata_value("hostname")
+        );
+        ensure!(
+            record.metadata_value("os_name") == Some("unknown"),
+            "agent sent os_name {:?} for a value made only of invisible format characters",
+            record.metadata_value("os_name")
+        );
+        ensure!(
+            record.metadata_value("domain").is_none(),
+            "agent sent domain {:?} for a value made only of invisible format characters",
+            record.metadata_value("domain")
+        );
+        Ok(())
+    })
+    .await
+}
+
 pub(crate) async fn a_make_before_break_on_renewal(ctx: Context) -> anyhow::Result<()> {
     // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §agent-renewal
     // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §channel-termination
@@ -6038,6 +6096,37 @@ mod tests {
 
     use super::*;
 
+    #[test]
+    fn agent_metadata_check_refuses_listed_format_characters_only() {
+        // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata
+        let metadata = |hostname: &str| {
+            let mut metadata = json!({
+                "hostname": hostname,
+                "os_name": "os",
+                "os_version": "1",
+                "arch": std::env::consts::ARCH,
+                "agent_version": "1",
+            });
+            // The check compares `machine_id` with the host's on Linux, so the fixture carries the host's.
+            if cfg!(target_os = "linux")
+                && let Ok(machine_id) = std::fs::read_to_string("/etc/machine-id")
+            {
+                metadata["machine_id"] = machine_id.trim().into();
+            }
+            metadata
+        };
+        validate_agent_metadata(&metadata("host\u{2065}name"), None).expect("U+2065 is outside the list");
+        for character in ['\u{00AD}', '\u{E007F}'] {
+            let refused = validate_agent_metadata(&metadata(&format!("host{character}name")), None)
+                .expect_err(&format!("U+{:04X} is listed", u32::from(character)));
+            assert!(
+                refused.to_string().contains("invisible format character"),
+                "U+{:04X} was refused for another reason: {refused}",
+                u32::from(character)
+            );
+        }
+    }
+
     #[test]
     fn key_names_require_this_runs_prefix_and_a_canonical_uuid() -> anyhow::Result<()> {
         // oracle: intent crates/device-key/INTENT.md §Key name

crates/agent-identity-conformance/src/client.rs

@@ -1,3 +1,4 @@
+use std::ops::RangeInclusive;
 use std::path::{Path, PathBuf};
 use std::sync::Arc;
 use std::time::Duration;
@@ -464,10 +465,124 @@ pub(crate) fn decoded_certificate(certificate: &str) -> anyhow::Result<Vec<u8>>
         .context("decode certificate")
 }
 
+/// The invisible format characters that no metadata value holds, as [CONTRACT.agent-identity.md] §3 "Metadata" lists them.
+///
+/// [CONTRACT.agent-identity.md]: ../../../docs/agent-identity/CONTRACT.agent-identity.md
+// oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata
+const INVISIBLE_FORMAT_CHARACTERS: [RangeInclusive<char>; 11] = [
+    '\u{00AD}'..='\u{00AD}',
+    '\u{061C}'..='\u{061C}',
+    '\u{180E}'..='\u{180E}',
+    '\u{200B}'..='\u{200F}',
+    '\u{202A}'..='\u{202E}',
+    '\u{2060}'..='\u{2064}',
+    '\u{2066}'..='\u{206F}',
+    '\u{FEFF}'..='\u{FEFF}',
+    '\u{FFF9}'..='\u{FFFB}',
+    '\u{E0001}'..='\u{E0001}',
+    '\u{E0020}'..='\u{E007F}',
+];
+
+pub(crate) fn is_invisible_format_character(character: char) -> bool {
+    INVISIBLE_FORMAT_CHARACTERS
+        .iter()
+        .any(|range| range.contains(&character))
+}
+
+/// The first and last code point of each range of [`INVISIBLE_FORMAT_CHARACTERS`], in order.
+///
+/// An implementation that holds the list as ranges goes wrong at their ends, so these are the code points worth sending.
+///
+/// A single code point is both ends of its range, and appears once.
+pub(crate) fn invisible_format_character_range_ends() -> Vec<char> {
+    let mut ends = INVISIBLE_FORMAT_CHARACTERS
+        .iter()
+        .flat_map(|range| [*range.start(), *range.end()])
+        .collect::<Vec<_>>();
+    ends.dedup();
+    ends
+}
+
+/// Code points that a metadata value may hold although an implementation could take them for invisible format characters.
+///
+/// They are each code point just outside a range of [`INVISIBLE_FORMAT_CHARACTERS`], which a range off by one refuses, and format characters (`Cf`) that the closed list leaves out, which a filter by general category refuses.
+pub(crate) fn unlisted_format_character_neighbors() -> Vec<char> {
+    let mut neighbors = INVISIBLE_FORMAT_CHARACTERS
+        .iter()
+        .flat_map(|range| [u32::from(*range.start()) - 1, u32::from(*range.end()) + 1])
+        .filter_map(char::from_u32)
+        .filter(|character| !is_invisible_format_character(*character))
+        .collect::<Vec<_>>();
+    // U+2065 is just outside both U+2060–U+2064 and U+2066–U+206F.
+    neighbors.dedup();
+    // oracle: spec Unicode 15.0.0 UnicodeData.txt §General_Category
+    // U+0600 ARABIC NUMBER SIGN, U+110BD KAITHI NUMBER SIGN and U+1D173 MUSICAL SYMBOL BEGIN BEAM are each `Cf`.
+    neighbors.extend(['\u{0600}', '\u{110BD}', '\u{1D173}']);
+    neighbors
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
+    #[test]
+    fn invisible_format_character_lists_match_the_contract() {
+        // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §metadata
+        assert_eq!(
+            invisible_format_character_range_ends(),
+            [
+                '\u{00AD}',
+                '\u{061C}',
+                '\u{180E}',
+                '\u{200B}',
+                '\u{200F}',
+                '\u{202A}',
+                '\u{202E}',
+                '\u{2060}',
+                '\u{2064}',
+                '\u{2066}',
+                '\u{206F}',
+                '\u{FEFF}',
+                '\u{FFF9}',
+                '\u{FFFB}',
+                '\u{E0001}',
+                '\u{E0020}',
+                '\u{E007F}',
+            ]
+        );
+        let neighbors = unlisted_format_character_neighbors();
+        assert_eq!(
+            neighbors,
+            [
+                '\u{00AC}',
+                '\u{00AE}',
+                '\u{061B}',
+                '\u{061D}',
+                '\u{180D}',
+                '\u{180F}',
+                '\u{200A}',
+                '\u{2010}',
+                '\u{2029}',
+                '\u{202F}',
+                '\u{205F}',
+                '\u{2065}',
+                '\u{2070}',
+                '\u{FEFE}',
+                '\u{FF00}',
+                '\u{FFF8}',
+                '\u{FFFC}',
+                '\u{E0000}',
+                '\u{E0002}',
+                '\u{E001F}',
+                '\u{E0080}',
+                '\u{0600}',
+                '\u{110BD}',
+                '\u{1D173}',
+            ]
+        );
+        assert!(!neighbors.into_iter().any(is_invisible_format_character));
+    }
+
     #[test]
     fn error_body_server_time_must_end_in_z() -> anyhow::Result<()> {
         // oracle: contract docs/agent-identity/CONTRACT.agent-identity.md §errors