sec(snapshot): integrity check uses public tag, not secret key — snapshots are forgeable

[chaliy]

Summary

The snapshot integrity check uses `SHA-256(BKSNAP01 || payload)` where `BKSNAP01` is a hardcoded constant in the source code. This means anyone with access to the code (which is public) can compute valid snapshot digests and forge arbitrary snapshots. The check only protects against accidental corruption, not intentional tampering.

Threat category: NEW — TM-SNAP (Snapshot Security) / extends TM-INT
Severity: Medium
Component: `crates/bashkit/src/snapshot.rs`, `compute_digest()` function

Root Cause

const INTEGRITY_TAG: &[u8; 8] = b"BKSNAP01";

fn compute_digest(payload: &[u8]) -> [u8; DIGEST_LEN] {
    let mut hasher = Sha256::new();
    hasher.update(INTEGRITY_TAG);  // Public constant, not a secret
    hasher.update(payload);
    let result = hasher.finalize();
    // ...
}

A real HMAC requires a secret key unknown to the attacker. The current implementation is functionally equivalent to a plain SHA-256 hash since the tag is public.

Steps to Reproduce

use sha2::{Sha256, Digest};

// Attacker creates malicious snapshot with injected state
let malicious_payload = serde_json::json!({
    "version": 1,
    "shell": {
        "variables": {"PATH": "/attacker/bin:/usr/bin"},
        "env": {"SECRET": "injected_value"},
        // ... crafted state
    },
    "vfs": null,
    "session_commands": 0,
    "session_exec_calls": 0
});
let json = serde_json::to_vec(&malicious_payload).unwrap();

// Compute valid digest using the public tag
let mut hasher = Sha256::new();
hasher.update(b"BKSNAP01");
hasher.update(&json);
let digest: [u8; 32] = hasher.finalize().into();

// Forge valid snapshot bytes
let mut forged = Vec::new();
forged.extend_from_slice(&digest);
forged.extend_from_slice(&json);

// This passes integrity check
let snapshot = Snapshot::from_bytes(&forged).unwrap(); // SUCCESS

Impact

• State injection: Attacker can inject arbitrary shell variables, environment, traps, aliases into a restored interpreter
• Sandbox escape setup: Injected traps or aliases could execute malicious code on next `exec()`
• Supply chain: If snapshots are transmitted over network or stored in shared locations, they can be replaced

Acceptance Criteria

• Replace hardcoded tag with HMAC using a caller-provided key, or document that snapshot integrity is NOT a security boundary
• If keyed: `Bash::snapshot(key: &[u8])` and `Bash::from_snapshot(data, key)`
• If accepted risk: Add prominent doc comment explaining forgability and when NOT to use snapshots
• Add test: forged snapshot with correct digest structure but wrong key is rejected (if keyed)
• Update threat model with snapshot security section

Context

Issue #977 (closed) added the SHA-256 digest check, but the implementation uses a public tag rather than a secret key, so it only prevents accidental corruption, not malicious forging.