Feature/tee agent

[michalpalkowski]

Summary by CodeRabbit

Release Notes

• New Features

  • Added TEE Agent HTTP API service for managing TEE VM lifecycle (start, stop, status, logs, health endpoints).
  • Added AMD SEV-SNP VM build infrastructure with reproducible build scripts and systemd service for deployment.
  • Added SNP cryptographic tools (snp-digest, snp-report, ovmf-metadata) for attestation and measurement operations.

• Documentation

  • Added comprehensive AMD SEV-SNP build and deployment guide.

• Chores

  • Updated build artifact ignore patterns for new SEV-SNP build outputs.

[coderabbitai]

📝 Walkthrough

Walkthrough

This pull request introduces a new TEE Agent HTTP API service for managing Katana VMs via QEMU with AMD SEV-SNP support, plus comprehensive build infrastructure for reproducible TEE component creation. Extends workspace members, adds relevant dependencies, and includes SNP utility tooling.

Changes

Cohort / File(s)	Summary
Build Configuration .dockerignore, .gitignore, Cargo.toml, crates/tee/Cargo.toml	Updated ignore patterns for vendor and AMDSEV directories; added workspace members bin/tee-agent and misc/AMDSEV/snp-tools; added sev dependency for SEV-SNP support.
TEE Agent HTTP API bin/tee-agent/Cargo.toml, bin/tee-agent/src/config.rs, bin/tee-agent/src/main.rs, bin/tee-agent/src/routes.rs, bin/tee-agent/src/vm.rs	New Rust binary providing HTTP endpoints for VM lifecycle management (start/stop/status/logs/health). Implements configuration parsing, route handlers, and VmManager for QEMU VM orchestration with SEV-SNP options, kernel cmdline construction, and graceful shutdown.
AMD SEV Build Scripts misc/AMDSEV/build.sh, misc/AMDSEV/build-config, misc/AMDSEV/build-ovmf.sh, misc/AMDSEV/build-kernel.sh, misc/AMDSEV/build-initrd.sh, misc/AMDSEV/build-qemu.sh, misc/AMDSEV/verify-build.sh	Orchestration and component build scripts for reproducible AMD SEV-SNP TEE builds. Includes config for pinned versions, QEMU builder, OVMF/kernel/initrd builders, and build verification with checksums.
AMD SEV Documentation & Deployment misc/AMDSEV/README.md, misc/AMDSEV/start-vm.sh, misc/AMDSEV/katana-tee-agent.service	Detailed documentation of SEV-SNP workflow, launch parameters, and troubleshooting; shell script for launching TEE VMs with serial logging; systemd service unit for tee-agent deployment.
SNP Tools & Utilities misc/AMDSEV/snp-tools/Cargo.toml, misc/AMDSEV/snp-tools/.cargo/config.toml, misc/AMDSEV/snp-tools/src/lib.rs, misc/AMDSEV/snp-tools/src/bin/snp-digest.rs, misc/AMDSEV/snp-tools/src/bin/snp-report.rs, misc/AMDSEV/snp-tools/src/bin/ovmf-metadata.rs	New Rust binary crate providing three CLI tools: snp-digest (compute SEV-SNP launch digests), snp-report (decode attestation reports with table/raw output), and ovmf-metadata (inspect OVMF sections and SEV metadata).
Musl Build Tooling scripts/build-musl.sh	New build script for statically linked Katana binary using musl with reproducible builds via SOURCE_DATE_EPOCH, auto-toolchain setup, and vendored dependency detection.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant Axum Server
    participant VmManager
    participant QEMU

    Client->>Axum Server: POST /start {fork_block, fork_provider, port}
    activate Axum Server
    Axum Server->>VmManager: start(StartRequest)
    activate VmManager
    
    alt VM Not Running
        VmManager->>VmManager: Validate boot components
        VmManager->>VmManager: Build kernel cmdline
        VmManager->>QEMU: Spawn QEMU with SEV-SNP config
        activate QEMU
        VmManager->>VmManager: Store RunningVm state (PID, timestamp, serial log)
        VmManager->>Axum Server: Return RPC URL
    else VM Already Running
        VmManager->>Axum Server: Return error
    end
    
    deactivate VmManager
    Axum Server->>Client: StartResponse {status, rpc_url, pid}
    deactivate Axum Server
    
    Note over Client,QEMU: Later: Client queries status or retrieves logs
    Client->>Axum Server: GET /status or GET /logs
    Axum Server->>VmManager: status() or logs(lines)
    VmManager->>VmManager: Read shared state (Mutex)
    VmManager->>Axum Server: Return VmStatus or LogsResponse
    Axum Server->>Client: JSON response

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 A TEE in the warren, so secure and bright,
With SEV-SNP shields to guard through the night,
VMs spring forth from QEMU's command,
Built once, reproduced across every land. 🏗️
The snp-tools scurry, digest and report,
Attestation verified in every fort! ✨

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The title 'Feature/tee agent' is vague and uses generic conventional prefixes without conveying meaningful information about the changeset's purpose or scope.	Consider using a more descriptive title that explains what the tee-agent does or its primary purpose, such as 'Add TEE Agent HTTP API for VM lifecycle management' or 'Implement QEMU VM manager for AMD SEV-SNP TEE environments'.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feature/tee-agent

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 13

🤖 Fix all issues with AI agents

In `@bin/tee-agent/src/config.rs`:
- Around line 1-80: Run rustfmt/cargo fmt on this file and commit the formatted
changes to resolve the CI rustfmt mismatch; specifically reformat the file that
defines the Config struct and its impl (symbols: Config, ovmf_path, kernel_path,
initrd_path, validate_boot_components) so whitespace/indentation and line
wrapping match the project's rustfmt rules, then add and commit the updated
file.

In `@bin/tee-agent/src/main.rs`:
- Line 13: Rustfmt is failing on the combined import line in main.rs; update the
axum import to match rustfmt expectations (e.g., reorder/nest or split the
import so Router and routing::{get, post} are formatted per rustfmt). Run
scripts/rust_fmt.sh --check (and then the formatter) to verify; modify the use
statement that references Router and routing::{get, post} so it conforms to the
project's rustfmt style.

In `@bin/tee-agent/src/routes.rs`:
- Around line 1-145: The file has rustfmt/style mismatches; run the formatter
(cargo fmt or rustfmt) and commit the resulting changes so CI passes.
Specifically, format this module containing AppState, the
StartResponse/StopResponse/ErrorResponse/LogsResponse/LogsQuery types and the
route handlers start_vm, stop_vm, get_status, get_logs, stop_vm and health to
fix spacing/indentation and import ordering issues reported by rustfmt.
- Around line 61-66: The current tracing::info call logs req.fork_provider raw
which may contain embedded API keys/tokens; change the log to avoid leaking
secrets by replacing req.fork_provider with either a redacted value or a parsed
safe component (e.g., host/scheme) — implement a small helper like
redact_fork_provider(&str) or parse the URL and return only the host/scheme or
"REDACTED" when sensitive, then call tracing::info!(..., fork_provider =
%redact_fork_provider(&req.fork_provider), ...); keep the rest of the log the
same.

In `@bin/tee-agent/src/vm.rs`:
- Around line 82-88: The kernel_cmdline string currently embeds
req.fork_provider (the full RPC URL) and is logged via info!, which can leak
credentials; fix by not logging the raw fork provider: construct kernel_cmdline
as you do (using req.fork_provider) but create a sanitized copy for logging
where you replace or mask req.fork_provider (e.g., with "<redacted>" or a masked
host) before calling info!, referencing the kernel_cmdline variable and
req.fork_provider so only the sanitized value is logged.
- Around line 9-10: Replace direct blocking calls to std::process::Child::wait()
inside the async stop() function (and the other blocking wait() usage around
lines 187-196) with tokio::task::spawn_blocking that runs the blocking wait()
and await the JoinHandle so the async runtime isn’t blocked; locate the stop()
implementation and any other functions calling Child::wait() and wrap the wait()
call in spawn_blocking(move || child.wait()).await and propagate errors as
before. Also sanitize the kernel cmdline logging by redacting the fork_provider
RPC URL when constructing/logging the kernel cmdline string (replace the
fork_provider value with a placeholder like "[REDACTED]" before calling
info!/warn!), ensuring you update the log site that currently prints the raw
kernel cmdline and any other places that log fork_provider.

In `@misc/AMDSEV/build-initrd.sh`:
- Around line 346-349: The initrd script currently attempts to format /dev/sda
with /sbin/mkfs.ext4 which may not exist in busybox-only initrd; modify the
block around the mount check to first test for an available formatter: if
/sbin/mkfs.ext4 exists use it, else if busybox provides mkfs.ext2 use that as a
fallback (and adjust the subsequent mount type to ext2), and if neither exists
log a clear error via the log function and fail gracefully without attempting to
run a missing binary; alternatively, add a conditional code path or config
switch to disable auto-formatting and document that storage must be
pre-formatted.

In `@misc/AMDSEV/build-qemu.sh`:
- Around line 38-44: The run_cmd function uses eval "$*" which permits command
injection and mishandles arguments with spaces; update run_cmd to avoid eval by
executing the received arguments directly (use the "$@" argument vector) and
propagate the exit status on failure, ensuring calls that pass DEST (from the
--prefix option) are safely executed; locate the run_cmd definition and its
callers (e.g., where DEST is passed) and replace the eval-based invocation with
a direct exec of "$@" and preserve the existing error message/exit behavior.

In `@misc/AMDSEV/build.sh`:
- Around line 127-149: The error logging currently echoes the exit code of the
test command rather than the build script because it runs the script then checks
`[ $? -ne 0 ]`; change each block that calls "${SCRIPT_DIR}/build-ovmf.sh" (when
BUILD_OVMF), "${SCRIPT_DIR}/build-kernel.sh" (when BUILD_KERNEL), and
"${SCRIPT_DIR}/build-initrd.sh" (when BUILD_INITRD) to use the pattern `if !
<script> args; then rc=$?; echo "<name> build failed: $rc"; exit $rc; fi` (i.e.,
use `if ! cmd; then` to capture the actual failure code into a variable and
log/exit with it) so the real exit code from the build script is preserved in
the message and exit.
- Around line 119-125: The test incorrectly uses an unquoted $INSTALL_DIR and
the obsolete -a operator; change the conditional to quote INSTALL_DIR and split
the test into two POSIX-safe checks like: [ -n "$INSTALL_DIR" ] && [ -d
"$INSTALL_DIR" ] || { ... } while keeping the prior IDIR assignment and the
mkdir -p call; update the three lines referencing INSTALL_DIR/IDIR so paths are
quoted (e.g., "$INSTALL_DIR" and "$IDIR") to handle spaces safely.

In `@misc/AMDSEV/README.md`:
- Around line 204-209: The fenced code block showing the error message
"qemu-system-x86_64: SEV: guest firmware hashes table area is invalid (base=0x0
size=0x0)" in misc/AMDSEV/README.md lacks a language hint; update that fenced
block to include a language label such as "text" or "console" (e.g., replace the
opening ``` with ```text) so markdownlint MD040 is satisfied and the
troubleshooting section no longer triggers the unlabeled-fence warning.

In `@misc/AMDSEV/snp-tools/Cargo.toml`:
- Around line 26-32: Update the sev git dependency in Cargo.toml to pin it to
the exact commit instead of using a moving branch; replace branch = "main" with
rev = "3d5c52ff7dcd3f67b14f77d96d76aae1eb9f7f8b" for the [dependencies.sev]
entry so the manifest explicitly references that commit while keeping
default-features = false and features = ["snp", "openssl"] unchanged.

In `@misc/AMDSEV/snp-tools/src/bin/snp-report.rs`:
- Around line 75-85: The stdin path uses io::stdin().read_to_string into a
String and then branches on args.binary, which fails for non‑UTF8 binary input;
change the logic to read raw bytes (use io::stdin().read_to_end into a Vec<u8>)
when args.binary is true (or read bytes first and convert to String only when
!args.binary), so replace the read_to_string call with a conditional read_to_end
for the binary branch (or read bytes and call String::from_utf8 when not
binary), updating the code around the buffer variable and the branch that
currently calls decode_hex(&buffer) to use the byte buffer and decode_hex on
text only.

🧹 Nitpick comments (7)

misc/AMDSEV/katana-tee-agent.service (1)

7-23: Consider tightening service hardening where feasible.

Running a network-facing service as root with hardening disabled increases blast radius. If root is only for device access, prefer a dedicated user with the right device permissions and re-enable basic hardening (e.g., NoNewPrivileges, ProtectSystem, ProtectHome).

🔒 Optional hardening tweaks (if compatible with your runtime needs)

-NoNewPrivileges=false
-ProtectSystem=false
-ProtectHome=false
+NoNewPrivileges=true
+ProtectSystem=full
+ProtectHome=read-only

bin/tee-agent/src/config.rs (1)

67-74: Consider validating boot component paths as files.

exists() will also accept directories named OVMF.fd, vmlinuz, or initrd.img, which can lead to confusing QEMU errors. Using is_file() makes the validation stricter and clearer.

♻️ Suggested tweak

-            if !path.exists() {
+            if !path.is_file() {

misc/AMDSEV/build-kernel.sh (2)

23-23: Unused variable SCRIPT_DIR.

The variable is defined but never used in this script. Consider removing it or using it (e.g., to source build-config from relative path).

Proposed fix

-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

Or if you want to use it to source build-config automatically:

 SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
+
+# Source build-config if not already sourced
+if [[ -z "${KERNEL_VERSION:-}" ]] && [[ -f "$SCRIPT_DIR/build-config" ]]; then
+    source "$SCRIPT_DIR/build-config"
+fi

---

42-48: run_cmd uses eval which could be avoided.

While the usage here is controlled, using eval can be error-prone. A simpler approach executes arguments directly.

Proposed simplification

 run_cmd() {
     echo "$*"
-    eval "$*" || {
+    "$@" || {
         echo "ERROR: $*"
         exit 1
     }
 }

Then invoke as run_cmd apt-get download "linux-image-unsigned-${KERNEL_VER}-generic" (without quoting the whole command as a string).

misc/AMDSEV/start-vm.sh (1)

53-53: Hardcoded fork parameters in kernel command line.

The KERNEL_CMDLINE contains hardcoded fork parameters (--fork.block,6177441,--fork.provider,https://pathfinder-sepolia.d.karnot.xyz/). This limits the script to a specific Sepolia testnet configuration.

Consider making these configurable via environment variables or command-line arguments for flexibility, or document that this is an example configuration.

Proposed refactor for configurability

+# Fork parameters (configurable)
+FORK_BLOCK="${FORK_BLOCK:-6177441}"
+FORK_PROVIDER="${FORK_PROVIDER:-https://pathfinder-sepolia.d.karnot.xyz/}"
+
 # Boot components
 OVMF_FILE="$BOOT_DIR/OVMF.fd"
 KERNEL_FILE="$BOOT_DIR/vmlinuz"
 INITRD_FILE="$BOOT_DIR/initrd.img"
-KERNEL_CMDLINE="console=ttyS0 katana.args=--http.addr,0.0.0.0,--http.port,5050,--tee.provider,sev-snp,--fork.block,6177441,--fork.provider,https://pathfinder-sepolia.d.karnot.xyz/"
+KERNEL_CMDLINE="console=ttyS0 katana.args=--http.addr,0.0.0.0,--http.port,5050,--tee.provider,sev-snp,--fork.block,${FORK_BLOCK},--fork.provider,${FORK_PROVIDER}"

misc/AMDSEV/build-initrd.sh (1)

27-27: Unused variable SCRIPT_DIR.

Same as in build-kernel.sh, this variable is defined but never used. Consider removing it or using it for relative path resolution.

Proposed fix

-SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"

misc/AMDSEV/build-ovmf.sh (1)

43-49: Avoid eval in run_cmd to prevent argument splitting/injection.

eval plus unquoted variables (Line 82, Line 129) can mis-handle spaces and allow unintended shell expansion. Prefer passing arguments as an array and invoking "$@", then quote variable expansions at call sites.

♻️ Proposed refactor

 run_cmd() {
     echo "$*"
-    eval "$*" || {
+    "$@" || {
         echo "ERROR: $*"
         exit 1
     }
 }
 
-BUILD_CMD="nice build -q --cmd-len=64436 -DDEBUG_ON_SERIAL_PORT=TRUE -n $(getconf _NPROCESSORS_ONLN) ${GCCVERS:+-t $GCCVERS} -a X64 -p OvmfPkg/AmdSev/AmdSevX64.dsc"
+BUILD_CMD=(nice build -q --cmd-len=64436 -DDEBUG_ON_SERIAL_PORT=TRUE -n "$(getconf _NPROCESSORS_ONLN)")
+if [[ -n "${GCCVERS:-}" ]]; then
+    BUILD_CMD+=(-t "$GCCVERS")
+fi
+BUILD_CMD+=(-a X64 -p OvmfPkg/AmdSev/AmdSevX64.dsc)
 ...
-    run_cmd $BUILD_CMD
+    run_cmd "${BUILD_CMD[@]}"

Also applies to: 72-83, 129-129

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Fix rustfmt mismatch to unblock CI.

The pipeline reports a rustfmt mismatch for this file. Please run the formatter (e.g., scripts/rust_fmt.sh or cargo fmt) and commit the result.

🧰 Tools

🪛 GitHub Actions: test

[error] 1-1: rustfmt formatting mismatch detected in config.rs. Run 'scripts/rust_fmt.sh --check' locally to see diffs and apply fixes.

🤖 Prompt for AI Agents

In `@bin/tee-agent/src/config.rs` around lines 1 - 80, Run rustfmt/cargo fmt on
this file and commit the formatted changes to resolve the CI rustfmt mismatch;
specifically reformat the file that defines the Config struct and its impl
(symbols: Config, ovmf_path, kernel_path, initrd_path, validate_boot_components)
so whitespace/indentation and line wrapping match the project's rustfmt rules,
then add and commit the updated file.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix rustfmt formatting issue.

The CI pipeline indicates a rustfmt mismatch. The import on line 13 appears to combine multiple items which rustfmt may want to split.

Proposed fix

-use axum::{Router, routing::{get, post}};
+use axum::routing::{get, post};
+use axum::Router;

Run scripts/rust_fmt.sh --check locally to see the exact formatting required.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	use axum::{Router, routing::{get, post}};
	use axum::routing::{get, post};
	use axum::Router;

🤖 Prompt for AI Agents

In `@bin/tee-agent/src/main.rs` at line 13, Rustfmt is failing on the combined
import line in main.rs; update the axum import to match rustfmt expectations
(e.g., reorder/nest or split the import so Router and routing::{get, post} are
formatted per rustfmt). Run scripts/rust_fmt.sh --check (and then the formatter)
to verify; modify the use statement that references Router and routing::{get,
post} so it conforms to the project's rustfmt style.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Fix rustfmt mismatch to unblock CI.

Pipeline failure indicates formatting mismatches in this file. Please run the formatter before merging (e.g., scripts/rust_fmt.sh --check / cargo fmt).

🧰 Tools

🪛 GitHub Actions: test

[error] 1-1: rustfmt formatting mismatch detected in routes.rs. Run 'scripts/rust_fmt.sh --check' locally to see diffs and apply fixes.

🤖 Prompt for AI Agents

In `@bin/tee-agent/src/routes.rs` around lines 1 - 145, The file has rustfmt/style
mismatches; run the formatter (cargo fmt or rustfmt) and commit the resulting
changes so CI passes. Specifically, format this module containing AppState, the
StartResponse/StopResponse/ErrorResponse/LogsResponse/LogsQuery types and the
route handlers start_vm, stop_vm, get_status, get_logs, stop_vm and health to
fix spacing/indentation and import ordering issues reported by rustfmt.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Do not log raw fork_provider (may include secrets).

RPC URLs frequently embed API keys or tokens; logging the full value can leak credentials.

🔒 Proposed fix

-    tracing::info!(
-        "Starting VM: fork_block={}, fork_provider={}, port={}",
-        req.fork_block,
-        req.fork_provider,
-        req.port
-    );
+    tracing::info!(
+        "Starting VM: fork_block={}, port={}, fork_provider=<redacted>",
+        req.fork_block,
+        req.port
+    );

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	tracing::info!(
	"Starting VM: fork_block={}, fork_provider={}, port={}",
	req.fork_block,
	req.fork_provider,
	req.port
	);
	tracing::info!(
	"Starting VM: fork_block={}, port={}, fork_provider=<redacted>",
	req.fork_block,
	req.port
	);

🤖 Prompt for AI Agents

In `@bin/tee-agent/src/routes.rs` around lines 61 - 66, The current tracing::info
call logs req.fork_provider raw which may contain embedded API keys/tokens;
change the log to avoid leaking secrets by replacing req.fork_provider with
either a redacted value or a parsed safe component (e.g., host/scheme) —
implement a small helper like redact_fork_provider(&str) or parse the URL and
return only the host/scheme or "REDACTED" when sensitive, then call
tracing::info!(..., fork_provider = %redact_fork_provider(&req.fork_provider),
...); keep the rest of the log the same.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the vm.rs file
find . -name "vm.rs" -path "*/tee-agent/*" | head -5

Repository: michalpalkowski/katana

Length of output: 91

---

🏁 Script executed:

# Read the vm.rs file to examine lines 9-10 and 187-196
wc -l ./bin/tee-agent/src/vm.rs

Repository: michalpalkowski/katana

Length of output: 95

---

🏁 Script executed:

# Read the imports and the critical section where the blocking call occurs
cat -n ./bin/tee-agent/src/vm.rs | head -20

Repository: michalpalkowski/katana

Length of output: 796

---

🏁 Script executed:

# Read the stop() function around lines 187-196
cat -n ./bin/tee-agent/src/vm.rs | sed -n '180,200p'

Repository: michalpalkowski/katana

Length of output: 1002

---

🏁 Script executed:

# Check around line 87 for kernel cmdline and credential logging
cat -n ./bin/tee-agent/src/vm.rs | sed -n '80,95p'

Repository: michalpalkowski/katana

Length of output: 814

---

🏁 Script executed:

# Check if task module is already imported elsewhere in the file
rg "use tokio::" ./bin/tee-agent/src/vm.rs

Repository: michalpalkowski/katana

Length of output: 89

---

🏁 Script executed:

# Verify the VM struct to understand the process field type
cat -n ./bin/tee-agent/src/vm.rs | sed -n '30,50p'

Repository: michalpalkowski/katana

Length of output: 711

---

Avoid blocking Child::wait() on the async runtime.

std::process::Child::wait() blocks the tokio worker thread in the async stop() function and can stall the server if QEMU is slow to exit. Use spawn_blocking to prevent blocking the async runtime.

Proposed fix (spawn_blocking)

-use tokio::sync::Mutex;
+use tokio::{sync::Mutex, task};
...
             // Wait for process to exit
-            match vm.process.wait() {
-                Ok(status) => info!("VM exited with status: {}", status),
-                Err(e) => warn!("Failed to wait for process: {}", e),
-            }
+            let mut process = vm.process;
+            match task::spawn_blocking(move || process.wait()).await {
+                Ok(Ok(status)) => info!("VM exited with status: {}", status),
+                Ok(Err(e)) => warn!("Failed to wait for process: {}", e),
+                Err(e) => warn!("Failed to join wait task: {}", e),
+            }

Also applies to: 187-196

Additionally, redact the RPC URL from kernel cmdline logs (line 87) to avoid leaking credentials. The fork_provider field is logged unfiltered in the kernel cmdline string.

🤖 Prompt for AI Agents

In `@bin/tee-agent/src/vm.rs` around lines 9 - 10, Replace direct blocking calls
to std::process::Child::wait() inside the async stop() function (and the other
blocking wait() usage around lines 187-196) with tokio::task::spawn_blocking
that runs the blocking wait() and await the JoinHandle so the async runtime
isn’t blocked; locate the stop() implementation and any other functions calling
Child::wait() and wrap the wait() call in spawn_blocking(move ||
child.wait()).await and propagate errors as before. Also sanitize the kernel
cmdline logging by redacting the fork_provider RPC URL when constructing/logging
the kernel cmdline string (replace the fork_provider value with a placeholder
like "[REDACTED]" before calling info!/warn!), ensuring you update the log site
that currently prints the raw kernel cmdline and any other places that log
fork_provider.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

cat -n misc/AMDSEV/build.sh | sed -n '115,130p'

Repository: michalpalkowski/katana

Length of output: 561

---

🌐 Web query:

POSIX shell test [ ] operator -a vs separate tests best practice

💡 Result:

In POSIX test / [ ... ], using -a / -o to combine expressions (AND/OR) is obsolete and can be ambiguous, especially once you get into 4+ arguments or user-supplied values that might look like operators (e.g., !, (, =, -n, etc.). POSIX explicitly notes that with more than 4 arguments, results are unspecified (unless you rely on the XSI extensions), and even then it’s easy to trigger syntax/parse surprises. [1][2]

Best practice for portable POSIX sh:

• Prefer separate tests combined with the shell’s && / ||:

  [ -n "$a" ] && [ -n "$b" ]
  [ -f "$x" ] || [ -f "$y" ]

  This is well-defined and is the conversion POSIX guidance and tools like ShellCheck recommend. [2][3]

• If you’re in a shell that supports [[ ... ]] (bash/ksh/zsh), prefer [[ ... ]] with && / || (not -a / -o), since it has clearer parsing rules:

  [[ -n $a && -n $b ]]

  [4][5]

Why avoid [ p -a q ] specifically:

• -a is overloaded historically (it can also mean “file exists” as a unary file test in some contexts/implementations), and POSIX/XSI marks -a/-o (and ( )) as obsolescent because the grammar makes many expressions ambiguously defined. [3][5]

Rule of thumb: in POSIX scripts, write one primary per [ ... ] and combine them with && / ||. [3][5]

Sources:
[1] POSIX test argument-count rules / unspecified results (Ubuntu POSIX manpage) (manpages.ubuntu.com)
[2] POSIX examples recommending || / && instead of -o / -a (Ubuntu POSIX manpage) (manpages.ubuntu.com)
[3] ShellCheck SC2166 rationale + POSIX obsolescence quote (shellcheck.net)
[4] Bash manual: [[ ... ]] uses && / || with defined precedence (gnu.org)
[5] Bash manpage noting POSIX deprecated -a/-o and recommending &&/|| (manpagez.com)

Citations:

• 1: https://manpages.ubuntu.com/manpages/focal/man1/test.1posix.html?utm_source=openai
• 2: https://manpages.ubuntu.com/manpages/lunar/man1/test.1posix.html?utm_source=openai
• 3: https://www.shellcheck.net/wiki/SC2166?utm_source=openai
• 4: https://www.gnu.org/software/bash/manual/html_node/Conditional-Constructs.html?utm_source=openai
• 5: https://www.manpagez.com/man/1/bash/?utm_source=openai

---

🌐 Web query:

bash unquoted variables spaces PATH issues shell quoting best practice

💡 Result:

Why unquoted variables break (spaces, globs, PATH)

In bash, unquoted parameter expansions and command substitutions undergo word splitting (on $IFS) and then pathname expansion (globbing). That’s why "$var" is safe but $var can turn into multiple arguments or unexpectedly expand * into filenames. [1][2]

Best-practice rules (bash)

1. Quote variable expansions unless you explicitly want splitting/globbing

   rm -- "$file"
   cp -- "$src" "$dst"
   printf '%s\n' "$var"

   (ShellCheck rule of thumb: “Double quote to prevent globbing and word splitting.”) [2]

2. Quote command substitutions

   name="$(get_name)"     # not: name=$(get_name)

   Unquoted $(...) is split/globbed just like unquoted variables. [3]

3. Use "$@" for forwarding arguments

   somefunc() { othercmd "$@"; }

   "$@" preserves argument boundaries; unquoted $@/$* does not. [4][2]

4. If you actually need “words”, use arrays (don’t store argument lists in strings)

   opts=(-j 5 -B)
   [[ $debug == yes ]] && opts+=(-d)
   make "${opts[@]}"

   This is the standard fix for cases where quoting a string would collapse multiple intended args into one. [2]

PATH-specific pitfalls & safe patterns

Don’t try to “split PATH” by echoing it unquoted

Unquoted expansions are split on $IFS (default includes spaces/tabs/newlines), not on :. To split on :, explicitly parse it:

IFS=: read -r -a path_parts <<< "$PATH"
for d in "${path_parts[@]}"; do
  printf '%s\n' "$d"
done

Word splitting behavior is defined by bash and controlled by $IFS. [1]

When setting PATH, quote the assignment

PATH="$HOME/bin:$PATH"
export PATH

Quoting prevents accidental interpretation if components ever contain whitespace or glob characters. (Also recommended as a “quote-safe variables” habit.) [5]

Quick checklist

• Default: quote ("$var", "$(cmd)", "$@")
• Need multiple args: arrays ("${arr[@]}"), not strings
• Need to split: set IFS + read -a/mapfile, don’t rely on implicit splitting [2][1]

Sources: Bash manual on word splitting [1], Bash manual on special parameters like "$@" [4], ShellCheck guidance on quoting and arrays [2][3][5].

Citations:
[1] https://www.gnu.org/software/bash/manual/html_node/Word-Splitting.html
[2] https://www.shellcheck.net/wiki/SC2086
[3] https://www.shellcheck.net/wiki/SC2046
[4] https://www.gnu.org/s/bash/manual/bash.html
[5] https://www.shellcheck.net/wiki/SC2248

---

Quote INSTALL_DIR and replace -a with separate test conditions.

Unquoted paths break on spaces, and -a in [ ] is POSIX-obsolescent with undefined behavior when combined with other operators. Use separate [ ] tests joined by &&.

Suggested fix

-mkdir -p $INSTALL_DIR
-IDIR=$INSTALL_DIR
-INSTALL_DIR=$(readlink -e $INSTALL_DIR)
-[ -n "$INSTALL_DIR" -a -d "$INSTALL_DIR" ] || {
+mkdir -p "$INSTALL_DIR"
+IDIR="$INSTALL_DIR"
+INSTALL_DIR="$(readlink -e "$INSTALL_DIR")"
+[ -n "$INSTALL_DIR" ] && [ -d "$INSTALL_DIR" ] || {
 	echo "Installation directory [$IDIR] does not exist, exiting"
 	exit 1
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	mkdir -p $INSTALL_DIR
	IDIR=$INSTALL_DIR
	INSTALL_DIR=$(readlink -e $INSTALL_DIR)
	[ -n "$INSTALL_DIR" -a -d "$INSTALL_DIR" ] || {
	echo "Installation directory [$IDIR] does not exist, exiting"
	exit 1
	}
	mkdir -p "$INSTALL_DIR"
	IDIR="$INSTALL_DIR"
	INSTALL_DIR="$(readlink -e "$INSTALL_DIR")"
	[ -n "$INSTALL_DIR" ] && [ -d "$INSTALL_DIR" ] || {
	echo "Installation directory [$IDIR] does not exist, exiting"
	exit 1
	}

🧰 Tools

🪛 Shellcheck (0.11.0)

[warning] 122-122: Prefer [ p ] && [ q ] as [ p -a q ] is not well defined.

(SC2166)

🤖 Prompt for AI Agents

In `@misc/AMDSEV/build.sh` around lines 119 - 125, The test incorrectly uses an
unquoted $INSTALL_DIR and the obsolete -a operator; change the conditional to
quote INSTALL_DIR and split the test into two POSIX-safe checks like: [ -n
"$INSTALL_DIR" ] && [ -d "$INSTALL_DIR" ] || { ... } while keeping the prior
IDIR assignment and the mkdir -p call; update the three lines referencing
INSTALL_DIR/IDIR so paths are quoted (e.g., "$INSTALL_DIR" and "$IDIR") to
handle spaces safely.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check if the file exists and examine the target lines
if [ -f "misc/AMDSEV/build.sh" ]; then
  echo "=== File exists. Checking line count ==="
  wc -l misc/AMDSEV/build.sh
  echo ""
  echo "=== Lines 120-155 of build.sh ==="
  sed -n '120,155p' misc/AMDSEV/build.sh
else
  echo "File not found at misc/AMDSEV/build.sh"
  find . -name "build.sh" -type f 2>/dev/null | head -5
fi

Repository: michalpalkowski/katana

Length of output: 1107

---

🏁 Script executed:

# Demonstrate the $? shadowing issue in shell
cat > /tmp/test_exit_code.sh << 'EOF'
#!/bin/bash

# Simulate the problematic pattern
echo "=== Problematic pattern (current code) ==="
false  # Simulates a failed build script
if [ $? -ne 0 ]; then
    echo "Command failed with exit code: $?" # This will print 0, not the original failure code!
fi

echo ""
echo "=== Correct pattern (if ! cmd) ==="
false  # Simulates a failed build script
if ! false; then
    # Inside this block, we can capture immediately
    echo "Command failed"
fi

echo ""
echo "=== Alternative: Capture $? immediately ==="
false  # Simulates a failed build script
exit_code=$?
if [ $exit_code -ne 0 ]; then
    echo "Command failed with exit code: $exit_code" # Correct!
fi
EOF

bash /tmp/test_exit_code.sh

Repository: michalpalkowski/katana

Length of output: 269

---

Capture build-script failures directly instead of reusing $?.

The current pattern prints the exit code from the [ $? -ne 0 ] test (which is 0 when the test succeeds) rather than the build script's actual failure code. This means error messages show misleading exit codes. Use if ! cmd; then to preserve the real failure code.

🐛 Proposed fix

 if [ $BUILD_OVMF -eq 1 ]; then
-	"${SCRIPT_DIR}/build-ovmf.sh" "$INSTALL_DIR"
-	if [ $? -ne 0 ]; then
-		echo "OVMF build failed: $?"
-		exit 1
-	fi
+	if ! "${SCRIPT_DIR}/build-ovmf.sh" "$INSTALL_DIR"; then
+		echo "OVMF build failed"
+		exit 1
+	fi
 fi
 
 if [ $BUILD_KERNEL -eq 1 ]; then
-	"${SCRIPT_DIR}/build-kernel.sh" "$INSTALL_DIR"
-	if [ $? -ne 0 ]; then
-		echo "Kernel build failed: $?"
-		exit 1
-	fi
+	if ! "${SCRIPT_DIR}/build-kernel.sh" "$INSTALL_DIR"; then
+		echo "Kernel build failed"
+		exit 1
+	fi
 fi
 
 if [ $BUILD_INITRD -eq 1 ]; then
-	"${SCRIPT_DIR}/build-initrd.sh" "$KATANA_BINARY" "$INSTALL_DIR/initrd.img"
-	if [ $? -ne 0 ]; then
-		echo "Initrd build failed: $?"
-		exit 1
-	fi
+	if ! "${SCRIPT_DIR}/build-initrd.sh" "$KATANA_BINARY" "$INSTALL_DIR/initrd.img"; then
+		echo "Initrd build failed"
+		exit 1
+	fi
 	# Copy katana binary to output directory
 	cp "$KATANA_BINARY" "$INSTALL_DIR/katana"
 	echo "Copied katana binary to $INSTALL_DIR/katana"
 fi

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if [ $BUILD_OVMF -eq 1 ]; then
	"${SCRIPT_DIR}/build-ovmf.sh" "$INSTALL_DIR"
	if [ $? -ne 0 ]; then
	echo "OVMF build failed: $?"
	exit 1
	fi
	fi
	if [ $BUILD_KERNEL -eq 1 ]; then
	"${SCRIPT_DIR}/build-kernel.sh" "$INSTALL_DIR"
	if [ $? -ne 0 ]; then
	echo "Kernel build failed: $?"
	exit 1
	fi
	fi
	if [ $BUILD_INITRD -eq 1 ]; then
	"${SCRIPT_DIR}/build-initrd.sh" "$KATANA_BINARY" "$INSTALL_DIR/initrd.img"
	if [ $? -ne 0 ]; then
	echo "Initrd build failed: $?"
	exit 1
	fi
	# Copy katana binary to output directory
	if [ $BUILD_OVMF -eq 1 ]; then
	if ! "${SCRIPT_DIR}/build-ovmf.sh" "$INSTALL_DIR"; then
	echo "OVMF build failed"
	exit 1
	fi
	fi
	if [ $BUILD_KERNEL -eq 1 ]; then
	if ! "${SCRIPT_DIR}/build-kernel.sh" "$INSTALL_DIR"; then
	echo "Kernel build failed"
	exit 1
	fi
	fi
	if [ $BUILD_INITRD -eq 1 ]; then
	if ! "${SCRIPT_DIR}/build-initrd.sh" "$KATANA_BINARY" "$INSTALL_DIR/initrd.img"; then
	echo "Initrd build failed"
	exit 1
	fi
	# Copy katana binary to output directory

🧰 Tools

🪛 Shellcheck (0.11.0)

[warning] 130-130: This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten.

(SC2319)

---

[warning] 138-138: This $? refers to a condition, not a command. Assign to a variable to avoid it being overwritten.

(SC2319)

🤖 Prompt for AI Agents

In `@misc/AMDSEV/build.sh` around lines 127 - 149, The error logging currently
echoes the exit code of the test command rather than the build script because it
runs the script then checks `[ $? -ne 0 ]`; change each block that calls
"${SCRIPT_DIR}/build-ovmf.sh" (when BUILD_OVMF), "${SCRIPT_DIR}/build-kernel.sh"
(when BUILD_KERNEL), and "${SCRIPT_DIR}/build-initrd.sh" (when BUILD_INITRD) to
use the pattern `if ! <script> args; then rc=$?; echo "<name> build failed:
$rc"; exit $rc; fi` (i.e., use `if ! cmd; then` to capture the actual failure
code into a variable and log/exit with it) so the real exit code from the build
script is preserved in the message and exit.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Add a language hint to the fenced code block.

markdownlint MD040 flags the unlabeled fence in the troubleshooting section. Add a language (e.g., text or console) to silence the warning.

✍️ Suggested fix

-```
+```text
 qemu-system-x86_64: SEV: guest firmware hashes table area is invalid (base=0x0 size=0x0)

</details>

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.20.0)</summary>

[warning] 207-207: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

In @misc/AMDSEV/README.md around lines 204 - 209, The fenced code block showing
the error message "qemu-system-x86_64: SEV: guest firmware hashes table area is
invalid (base=0x0 size=0x0)" in misc/AMDSEV/README.md lacks a language hint;
update that fenced block to include a language label such as "text" or "console"
(e.g., replace the opening withtext) so markdownlint MD040 is satisfied
and the troubleshooting section no longer triggers the unlabeled-fence warning.

</details>

<!-- fingerprinting:phantom:medusa:eagle -->

<!-- This is an auto-generated comment by CodeRabbit -->

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, check if the file exists and read the relevant lines
cat -n misc/AMDSEV/snp-tools/Cargo.toml | sed -n '20,35p'

Repository: michalpalkowski/katana

Length of output: 597

---

🏁 Script executed:

# Check if Cargo.lock exists and search for sev dependency info
if [ -f "misc/AMDSEV/snp-tools/Cargo.lock" ]; then
  echo "=== Cargo.lock found ===" 
  rg -n 'name = "sev"' misc/AMDSEV/snp-tools/Cargo.lock -A 5
else
  echo "No Cargo.lock found in misc/AMDSEV/snp-tools/"
  # Check parent directory
  if [ -f "Cargo.lock" ]; then
    echo "=== Cargo.lock found at root ==="
    rg -n 'name = "sev"' Cargo.lock -A 5
  fi
fi

Repository: michalpalkowski/katana

Length of output: 593

---

🏁 Script executed:

# Check the directory structure to understand the project layout
ls -la misc/AMDSEV/snp-tools/ | head -20

Repository: michalpalkowski/katana

Length of output: 340

---

Pin the sev git dependency to a fixed commit in Cargo.toml.

Using branch = "main" creates a moving reference that is not explicitly pinned in the manifest. While the root Cargo.lock currently locks this to commit 3d5c52ff7dcd3f67b14f77d96d76aae1eb9f7f8b, relying solely on Cargo.lock is fragile for a security-sensitive project. Update Cargo.toml to explicitly specify the commit via rev to make the dependency choice clear and reproducible independent of lock files.

🔒 Suggested change

 [dependencies.sev]
 git = "https://github.com/virtee/sev"
-branch = "main"
+rev = "3d5c52ff7dcd3f67b14f77d96d76aae1eb9f7f8b"
 default-features = false
 features = ["snp", "openssl"]

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	# Use git dependency because crates.io v7.1.0 has rdrand as non-optional.
	# The git version has rdrand behind the "sev" feature flag, allowing builds on non-x86_64.
	[dependencies.sev]
	git = "https://github.com/virtee/sev"
	branch = "main"
	default-features = false
	features = ["snp", "openssl"]
	[dependencies.sev]
	git = "https://github.com/virtee/sev"
	rev = "3d5c52ff7dcd3f67b14f77d96d76aae1eb9f7f8b"
	default-features = false
	features = ["snp", "openssl"]

🤖 Prompt for AI Agents

In `@misc/AMDSEV/snp-tools/Cargo.toml` around lines 26 - 32, Update the sev git
dependency in Cargo.toml to pin it to the exact commit instead of using a moving
branch; replace branch = "main" with rev =
"3d5c52ff7dcd3f67b14f77d96d76aae1eb9f7f8b" for the [dependencies.sev] entry so
the manifest explicitly references that commit while keeping default-features =
false and features = ["snp", "openssl"] unchanged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

--binary stdin path fails on non‑UTF8 input.

When --binary is set, stdin is still read via read_to_string, which errors on arbitrary binary data. Read raw bytes instead to make --binary work as intended.

🐛 Suggested fix

-        // Read from stdin
-        let mut buffer = String::new();
-        io::stdin()
-            .read_to_string(&mut buffer)
-            .map_err(|e| format!("Failed to read stdin: {}", e))?;
-
-        if args.binary {
-            Ok(buffer.into_bytes())
-        } else {
-            decode_hex(&buffer)
-        }
+        // Read from stdin
+        if args.binary {
+            let mut data = Vec::new();
+            io::stdin()
+                .read_to_end(&mut data)
+                .map_err(|e| format!("Failed to read stdin: {}", e))?;
+            Ok(data)
+        } else {
+            let mut buffer = String::new();
+            io::stdin()
+                .read_to_string(&mut buffer)
+                .map_err(|e| format!("Failed to read stdin: {}", e))?;
+            decode_hex(&buffer)
+        }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	// Read from stdin
	let mut buffer = String::new();
	io::stdin()
	.read_to_string(&mut buffer)
	.map_err(|e| format!("Failed to read stdin: {}", e))?;
	if args.binary {
	Ok(buffer.into_bytes())
	} else {
	decode_hex(&buffer)
	}
	// Read from stdin
	if args.binary {
	let mut data = Vec::new();
	io::stdin()
	.read_to_end(&mut data)
	.map_err(|e| format!("Failed to read stdin: {}", e))?;
	Ok(data)
	} else {
	let mut buffer = String::new();
	io::stdin()
	.read_to_string(&mut buffer)
	.map_err(|e| format!("Failed to read stdin: {}", e))?;
	decode_hex(&buffer)
	}

🤖 Prompt for AI Agents

In `@misc/AMDSEV/snp-tools/src/bin/snp-report.rs` around lines 75 - 85, The stdin
path uses io::stdin().read_to_string into a String and then branches on
args.binary, which fails for non‑UTF8 binary input; change the logic to read raw
bytes (use io::stdin().read_to_end into a Vec<u8>) when args.binary is true (or
read bytes first and convert to String only when !args.binary), so replace the
read_to_string call with a conditional read_to_end for the binary branch (or
read bytes and call String::from_utf8 when not binary), updating the code around
the buffer variable and the branch that currently calls decode_hex(&buffer) to
use the byte buffer and decode_hex on text only.