AkurAI Build
Sign in with AkurAI ID
Menu

AkurAI-Build

public

Latest change 1c2159692a31765cd66ed709791ba11468054873 - Initial commit: bunfork v0.1.0 source tree by Olafur Bui

Pass 8 verdict: not release-safe for hostile local users or unattended recovery. The strongest risks are deployment replacement, restore TOCTOU, secret-equivalence checking, and HTTP error classification.

Must-fix, ranked:

1. **P0 — `deploy --force` ownership is forgeable and replacement is not durable.**  
   `src/main.rs:401-413,483-533` accepts a small self-asserted JSON manifest plus predictable directory names; an attacker who can place those files can make `--force` rename and recursively replace an arbitrary direct-child directory. The code also does not fsync the staged tree, parent, or replacement, and leaves PID-derived `*-previous-*` releases indefinitely (`src/main.rs:389-391,452-471`).

   Smallest fix: replace the descriptive manifest with a cryptographically or digest-bound ownership manifest covering every regular file; reject reserved paths (`.git`, `src`, `data`, `target`, `backups`) regardless of contents; stage with `create_new`, fsync files/directories, atomically rename, and retain exactly one verified previous release with bounded cleanup.

   Test: sentinel files in `.git`, `src`, `data`, and a forged-manifest directory must survive failed deployment; valid prior releases must replace successfully and recover after simulated rename failure.

2. **P0 — restore has symlink/hard-link TOCTOU exposure.**  
   `restore_database` validates `backup` and `live`, then later reopens/copies them (`src/db.rs:410-427,461-477`). A local attacker can swap the validated backup for a symlink or hard link between checks and `open_read_only`/`fs::copy`. `open_for_migration` also calls `prepare_database_path`, which does not reject hard links (`src/db.rs:58-64`; contrast `validate_existing_file:888-902`).

   Smallest fix: open and retain file descriptors using `O_NOFOLLOW`, verify inode/device/nlink from those descriptors, and copy from the opened descriptor; apply the same nlink check to migration-created/existing databases. Prefer directory-relative `openat`-style operations where available.

   Tests: race replacement with symlink and hard link during restore/migrate; assert no outside file is read or modified.

3. **P0 — equivalent secret encodings bypass separation.**  
   Startup compares textual strings (`src/main.rs:216-220`), while decoding accepts upper- and lowercase hex (`src/db.rs:729-750`). The same 32 bytes can therefore be used as both database key and API token with different casing. The existing test explicitly confirms case-insensitive keys (`src/db.rs` test `key_hex_is_case_insensitive`).

   Smallest fix: decode both secrets and compare the 32-byte values with constant-time equality. Add uppercase/lowercase equivalent-key rejection and whitespace-normalization cases.

4. **P1 — client input failures become HTTP 500.**  
   Vector dimension, NaN/infinite values, invalid IDs, limits, content size, and search-budget failures are returned through `ApiError::internal` (`src/server.rs:313-324,343-348`; validation lives in `src/db.rs:113-141,597-680`). This leaks an incorrect operational signal and invites retry storms.

   Smallest fix: classify validation errors as 400/413, budget exhaustion as 422 or 429, malformed JSON as 400, and database failures as 500. Preserve generic public messages.

   Tests: malformed vectors, zero norm, oversized content, invalid IDs, bad limits, and budget overflow must return stable 4xx responses.

5. **P1 — subprocess execution inherits the entire environment.**  
   `deploy` invokes `cargo` through `ProcessCommand` without environment reduction (`src/main.rs:331-334,564-573,577-590`). This is not shell injection, but build scripts and Cargo configuration can consume inherited secrets, proxies, credentials, or hostile variables.

   Smallest fix: use an explicit environment allowlist, explicit `--manifest-path`, fixed working directory, and record the resolved tool path/version. Test with a secret environment variable and a build script proving it is absent.

6. **P1 — schema validation is weaker than the operational claim.**  
   `validate_application_schema` checks application/user versions, migration ledger, and presence of two tables, but not exact columns, constraints, indexes, triggers, or unexpected objects (`src/db.rs:634-680`). A tampered database can satisfy the current acceptance test while violating the intended schema.

   Smallest fix: compare normalized `sqlite_schema` definitions against the embedded migration contract, or add explicit PRAGMA/table/index checks. Test extra tables, altered constraints, missing indexes, and changed migration SQL.

7. **P2 — deployment/service instructions are inconsistent.**  
   The copied unit uses `%h/.local/lib/bunfork` (`deploy/bunfork.service:7-9`), while README instructs running from `dist` (`README.md:91-100`). The unit also has no `NoNewPrivileges`, capability restrictions, filesystem protections, or explicit secret environment policy. The default `0.0.0.0:3100` is correctly preserved (`src/main.rs:20`, README:19, service:8).

   Smallest fix: make the unit paths match the documented install path and add narrowly compatible systemd hardening; test installation from an arbitrary working directory and SIGTERM shutdown.

Other findings:

- Backup publication correctly refuses overwrite and validates the encrypted result, but uses path checks followed by path operations (`src/db.rs:265-315`); apply the same descriptor/inode discipline.
- Restore preserves a `.pre-restore-*` copy, but there is no retention, verification, or recovery policy after installation (`src/db.rs:461-477`).
- Shutdown signal handling exists and Axum graceful shutdown is wired (`src/server.rs:193-207,735-756`), but there is no black-box test proving in-flight writes finish before exit.
- Public/page trees reject symlinks and hidden entries, but recursive regular-file checks are incomplete for special files in `validate_public_tree` (`src/server.rs:706-724`).
- Tenant/model are configuration, not authorization; README correctly states that boundary (`README.md:110-113`).

Outside the guarantee: encryption does not protect a live process, unlocked key, memory, backups without key custody, malicious root, compromised service account, compromised TLS proxy, or an attacker able to modify the binary and service files. HTTP remains plaintext unless deployed behind trusted TLS. No remote backup, key rotation, per-user authorization, rate limiting, or Windows filesystem hardening is guaranteed.