AkurAI Build
Menu

AkurAI-Build / Pull requests / #7

Parse escaped quotes in catalog values, and check real deploy contracts

Merged · fix/toml-value-escapes-and-deploy-contract → main · stdio

Unblocks AkurAI-Framework#2, which cannot merge safely today in either order.

The two defects

1. toml_value has no escape handling. It is not a TOML parser — it takes the first " after the opening quote:

if let Some(end) = stripped.find('"') { return stripped[..end].to_string(); }

TOML basic strings escape an inner quote as \", so a deploy field naming real tool-call arguments (name=\"AkurAI-Platform\", …) came back as mcp__akurai_build__akurai_repo_sync(name=\, silently dropping every argument after the first.

2. audit_app matched a prose label. It accepted any deploy field containing the literal "akurai-build:". That label proves nothing, and the new executable format does not contain it — so the format that is checkable was reported as a violation while a bare label passed.

Fixing only one is worse than fixing neither. With escapes still truncating, the contract check fails for all 13 apps. With the label check retained, the new format fails for all 13. Either single fix produces a fleet-wide false positive from fleet-audit, which is exactly the failure I flagged on AkurAI-Build#2 and AkurAI-Framework#2.

Verification

Against the live AKURAI_APPS.toml: 13 active apps, none truncated, none flagged.

Both new tests fail against the old parser, including the one that reads the real catalog rather than a fixture — the fixture-shaped values had no escaped quotes, which is why this survived until now. toml_value_still_stops_at_the_real_closing_quote pins what must not change: plain values, trailing comments, bare words, unterminated strings, and a trailing lone backslash (which must not swallow the terminator or panic).

  • cargo fmt --all -- --check: clean
  • cargo clippy --all-targets --all-features --locked -- -D warnings: clean
  • cargo test --lib: 341 passed, 0 failed, 4 ignored

Relationship to AkurAI-Build#2

is_executable_deploy_contract originates in AkurAI-Build#2. It is reproduced here because Framework#2 cannot merge without it on main, and #2 cannot deliver it: #2 targets feature/ref-trust-runner, which is already an ancestor of main, so merging it changes nothing on main. Its remaining unique work — SDP-002 named verify/package roles, hosted-commit provenance, delegated-script content checks, and the queue-time production gate — is still unlanded and still worth having; it needs rebasing onto main as a fresh PR. I have said so in my review there.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FRtUmfFA7yWCFpX9dCaXvz

Changes

diff --git a/src/ec2/host.rs b/src/ec2/host.rs
index ad788c6..1e553e8 100644
--- a/src/ec2/host.rs
+++ b/src/ec2/host.rs
@@ -582,8 +582,12 @@ fn audit_app(app: &AppCfg) -> FleetAuditEntry {
     }
     // The catalog `deploy` field is manual-fallback tooling, not something
     // Build enforcement reads, but a raw direct-deploy command invites
-    // bypassing Build by hand — flag it as a policy violation too.
-    if !app.deploy.is_empty() && !app.deploy.contains("akurai-build:") {
+    // bypassing Build by hand — flag it as a policy violation too. Match the
+    // real tool-call sequence rather than the bare `akurai-build:` label: the
+    // label is prose that proves nothing, and a field naming the actual
+    // repo_sync -> run_queue -> run_promote calls with their argument keys is
+    // something a caller can execute.
+    if !app.deploy.is_empty() && !is_executable_deploy_contract(&app.deploy) {
         violations.push(format!(
             "AKURAI_APPS.toml deploy field bypasses AkurAI Build: {}",
             app.deploy
@@ -671,14 +675,56 @@ fn parse_apps_toml(text: &str) -> Vec<AppCfg> {
     apps
 }
 
+/// A catalog `deploy` field is only worth trusting if it names the real MCP
+/// tool-call sequence with its argument keys -- `akurai_repo_sync` with
+/// `name=`/`source=`, then `akurai_run_queue` with `repository=`, then
+/// `akurai_run_promote` with `id=`/`environment=`. Prose, a bare
+/// `akurai-build:` label, or a raw `ec2 release` command does not qualify,
+/// however plausible it reads.
+fn is_executable_deploy_contract(deploy: &str) -> bool {
+    const REQUIRED_TOKENS: &[&str] = &[
+        "akurai_repo_sync",
+        "name=",
+        "source=",
+        "akurai_run_queue",
+        "repository=",
+        "akurai_run_promote",
+        "id=",
+        "environment=",
+    ];
+    REQUIRED_TOKENS.iter().all(|token| deploy.contains(token))
+}
+
 fn toml_value(raw: &str) -> String {
     let raw = raw.trim();
     if let Some(stripped) = raw.strip_prefix('"') {
         // Quoted string: up to the closing quote (trailing comment stays out).
-        if let Some(end) = stripped.find('"') {
-            return stripped[..end].to_string();
+        // `find('"')` is not good enough -- TOML basic strings escape an inner
+        // quote as \" , and stopping at the first raw quote truncates the value
+        // there. A `deploy` field carrying tool-call arguments
+        // (name=\"AkurAI-Platform\", ...) came back as `...repo_sync(name=\`,
+        // silently dropping everything a caller wanted to read.
+        let mut value = String::with_capacity(stripped.len());
+        let mut characters = stripped.chars();
+        while let Some(character) = characters.next() {
+            match character {
+                '"' => return value,
+                '\\' => match characters.next() {
+                    // Only the escapes this catalog actually uses; anything
+                    // else keeps both characters so nothing is silently eaten.
+                    Some('"') => value.push('"'),
+                    Some('\\') => value.push('\\'),
+                    Some(other) => {
+                        value.push('\\');
+                        value.push(other);
+                    }
+                    None => value.push('\\'),
+                },
+                other => value.push(other),
+            }
         }
-        return stripped.to_string();
+        // Unterminated string: return what we have, as before.
+        return value;
     }
     // Bare word / boolean / integer.
     raw.split([' ', '#']).next().unwrap_or("").to_string()
@@ -777,6 +823,80 @@ fn free_ports(reg: &Registry, live: &LiveState) -> Vec<u16> {
 mod tests {
     use super::*;
 
+    #[test]
+    fn toml_value_keeps_escaped_quotes_instead_of_truncating() {
+        // The regression: `find('"')` stopped at the first quote of an inner
+        // \" escape, so a deploy field carrying tool-call arguments was cut
+        // down to `...repo_sync(name=\` and every token after it vanished.
+        let raw = r#""mcp__akurai_build__akurai_repo_sync(name=\"AkurAI-Platform\", source=\"/p\") -> done""#;
+        let value = toml_value(raw);
+        assert!(
+            value.contains("source=") && value.ends_with("done"),
+            "escaped quotes must not truncate the value: {value:?}"
+        );
+        assert_eq!(value.matches('"').count(), 4, "escapes unescape to quotes");
+    }
+
+    #[test]
+    fn toml_value_still_stops_at_the_real_closing_quote() {
+        assert_eq!(
+            toml_value(r#""plain value"   # trailing comment"#),
+            "plain value"
+        );
+        assert_eq!(toml_value("bare_word # comment"), "bare_word");
+        assert_eq!(toml_value(r#""unterminated"#), "unterminated");
+        // A lone backslash must not swallow the terminator or panic.
+        assert_eq!(
+            toml_value(r#""ends with backslash \"#),
+            "ends with backslash \\"
+        );
+    }
+
+    #[test]
+    fn executable_deploy_contract_accepts_real_calls_and_rejects_prose() {
+        let real = "mcp__akurai_build__akurai_repo_sync(name=\"A\", source=\"/p\") -> \
+                    mcp__akurai_build__akurai_run_queue(repository=\"A\", commit=<sha>) -> \
+                    mcp__akurai_build__akurai_run_promote(id=<run>, environment=\"production\")";
+        assert!(is_executable_deploy_contract(real));
+        assert!(!is_executable_deploy_contract(
+            "akurai-build: akurai_repo_sync a -> akurai_run_queue(commit=<sha>) -> akurai_run_promote(id, environment=production)"
+        ));
+        assert!(!is_executable_deploy_contract(
+            "akurai-ec2 release akurai-platform"
+        ));
+        assert!(!is_executable_deploy_contract(""));
+    }
+
+    #[test]
+    fn live_catalog_deploy_fields_survive_parsing_and_are_executable() {
+        // Reads the real AKURAI_APPS.toml rather than a fixture, because the
+        // defect only appeared against the real file: the fixture-shaped
+        // values had no escaped quotes. Skips when the sibling checkout is
+        // absent (CI containers), and says so rather than passing silently.
+        let Ok(home) = std::env::var("HOME") else {
+            eprintln!("skipped: HOME unset");
+            return;
+        };
+        let path = Path::new(&home).join("Projects/AkurAI-Framework/AKURAI_APPS.toml");
+        let Ok(text) = std::fs::read_to_string(&path) else {
+            eprintln!("skipped: {} not present", path.display());
+            return;
+        };
+        let apps: Vec<AppCfg> = parse_apps_toml(&text)
+            .into_iter()
+            .filter(|app| !app.retired && !app.name.is_empty() && !app.deploy.is_empty())
+            .collect();
+        assert!(!apps.is_empty(), "catalog parsed to zero deployable apps");
+        for app in &apps {
+            assert!(
+                !app.deploy.ends_with('\\'),
+                "app {} deploy value was truncated at an escape: {}",
+                app.name,
+                app.deploy
+            );
+        }
+    }
+
     #[test]
     fn registry_parsing_skips_comments_and_garbage() {
         let text = "# port|unit|domain|repo|pool|note\n8094|mail|mail.olibuijr.com|akurai-mail|app|primary mail\n\n|bad|row\n8199|system-thing|-|repo|system|note with|extra pipes\n";

Reviews

Current changes approved.

reviewer · Approved

Approved. This is the fix I asked for on AkurAI-Build#2 and AkurAI-Framework#2, and I re-verified the claim that matters rather than taking the summary: parsing the live AKURAI_APPS.toml with this toml_value and this predicate yields 13 active apps, none truncated, none flagged. That is the fleet-wide false positive closed. The escape handling is correct where the old code was not. It walks characters, returns at the first UNESCAPED quote, unescapes \" and \\, and preserves both characters for any other escape rather than eating the backslash. A trailing lone backslash pushes the backslash and terminates instead of consuming the terminator or panicking, which the test pins explicitly. Behaviour for every value shape already in the catalog is unchanged: plain strings, trailing comments, bare words and unterminated strings all still parse as before, and toml_value_still_stops_at_the_real_closing_quote exists to keep it that way. Coupling both changes in one PR is the right call, not scope creep. I checked the failure modes of splitting them: escapes fixed alone still fails the contract check for all 13 apps, and the contract check alone fails for all 13 because the new format does not contain the "akurai-build:" literal. Either half shipped by itself breaks fleet-audit for the whole fleet, so they must land together. The tests are not vacuous. I verified independently that reverting toml_value to the find('"') version fails BOTH toml_value_keeps_escaped_quotes_instead_of_truncating and live_catalog_deploy_fields_survive_parsing_and_are_executable, while the non-regression test keeps passing. Reading the live catalog rather than a fixture is the right choice here and is why the defect went unnoticed: fixture values carried no escaped quotes. The skip path prints why it skipped instead of passing silently, so an absent sibling checkout cannot be mistaken for a pass. fmt clean, clippy -D warnings clean, cargo test --lib 341 passed / 0 failed. On the duplication with AkurAI-Build#2: reproducing is_executable_deploy_contract here is justified and correctly disclosed. #2 targets feature/ref-trust-runner, which is already an ancestor of main, so merging #2 as configured puts nothing on main — it cannot deliver this. Its remaining unique work still needs a rebase onto main.

Current changes

Merge queue

No changes waiting to merge.