diff --git a/CHANGELOG.md b/CHANGELOG.md index bfc62a0..1b85f1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 --- +## [Unreleased] + +### Added + +- **Share a whole project via relay** (issue #4) — one send + one receive now carries a project's structure (environments, `isDefault`) *and* the decrypted values of every item they reference, for however many environments are selected, in a single encrypted relay round-trip. Previously the same outcome took an export-template + relay-send-per-environment workaround. + - **Protocol**: `ProjectBundle`/`EnvironmentBundle`/`ProjectBundleVar` in `share/relay.rs` (`kind: "project"`, `version: 1`, checked on decrypt). Items are deduped by name and hoisted to the bundle root, so an item linked into 3 environments produces one bundled item, not three. Never carries `paths` (machine-specific, dropped) or a `literal` value field. + - **REST**: `POST /projects/:id/relay/send` (body `{environment_ids}`), `POST /projects/relay/receive` (body `{code, passphrase, project_name_override?}`). + - **Tauri**: `project_relay_send(project_id, environment_ids)`, `project_relay_receive(code, passphrase, project_name_override?)`. + - **CLI**: `crypt-env project share --id N --envs a,b [--yes]`, `crypt-env project receive --code X --passphrase Y [--as NAME]`. `--envs` omitted defaults to the default environment only; `--envs all` opts in explicitly. + - **GUI**: "SHARE PROJECT" / "RECEIVE PROJECT" on the Projects screen, with a per-environment key manifest (KEY → item name, never values) shown before upload and a rename prompt on a name collision. + - **Receive semantics**: always creates a **new** project — never merges into an existing one. A case-insensitive project-name collision is a hard error (`409 CONFLICT` over REST), retryable with an override name. The whole receive is one transaction (all-or-nothing). Received items are owned by the new project only, never `isGlobal`. + - **No MCP tool** — deliberately not exposed to MCP; see `docs/reference.md`'s Notes for the reasoning. + +### Removed (Breaking) + +- **Legacy workspace relay endpoints and MCP tools**, dead since the Projects/Environments migration: they read/wrote the frozen `workspaces`/`workspace_vars` tables that no product surface displays post-migration, so any import through them landed as ownerless, invisible items. + - REST: `POST /workspaces/:id/relay/send`, `POST /workspaces/relay/receive`. + - MCP tools: `crypt_env_share_workspace_send`, `crypt_env_share_workspace_receive`. + - `WorkspaceBundle`/`WorkspaceBundleVar`/`encrypt_workspace`/`decrypt_workspace` deleted from `share/relay.rs`. The underlying `workspaces`/`workspace_vars`/`workspace_paths` tables and their `db` accessors are **kept** — they remain the one-time migration backfill source for pre-migration installs. + - Not aliased: both endpoints were already documented as legacy/broken, and their one shipped consumer (the MCP server) is updated in the same change. + +--- + ## [1.0.2] - 2026-08-04 ### Changed diff --git a/docs/reference.md b/docs/reference.md index 54be7fa..84dca4d 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -10,8 +10,8 @@ Authentication: Header `X-Vault-Token` containing either a session token (from P |--------|----------|------|-------------| | POST | /unlock | none | Derives AES-GCM key from master password + Argon2 salt, generates 16-byte session token with configurable TTL | | GET | /health | none | Returns version, status, vault_locked bool, mcp_token_configured. No longer returns item_count (removed — see Notes) | -| GET | /items | token | List items (redacted — no secret values), **scoped**: requires `environment_id`, or `project`+`environment` (case-insensitive names) query params — 422 `VALIDATION_ERROR` if unresolvable. Returns only items linked in that environment's `environment_vars`. `type`/`category`/`search` filters apply on top | -| POST | /items | token | Create item, **scoped** (same query params as GET /items). Validates: name (req, max 255), type (one of: secret/credential/link/note/command), value (req non-empty). Body accepts optional `key` (environment-var key, defaults to `name`). Creates the item, owns it in the resolved project, links it into the resolved environment under `key`. Caller-supplied `isGlobal` is ignored — items created this way are always `isGlobal:false`. Encrypts with AES-GCM before storing. Returns 422 on validation/scope failure | +| GET | /items | token | List items (redacted — no secret values), **scoped**: requires `environment_id`, or `project`+`environment` (case-insensitive names) query params — 422 `VALIDATION_ERROR` if unresolvable. **Discovery endpoint** — see "Global items and scope" below: accepts `include_global=true\|false\|only` (default `true`), unioning in reusable global items not linked into this environment by default. Every item carries `isGlobal` and `linked`. `type`/`category`/`search` filters apply on top of the union | +| POST | /items | token | Create item, **scoped** (same query params as GET /items). Validates: name (req, max 255), type (one of: secret/credential/link/note/command), value (req non-empty). Body accepts optional `key` (environment-var key, defaults to `name`). Accepts `?on_conflict=update\|replace\|error` (default `update`; invalid value → 422 `VALIDATION_ERROR`). On no collision: creates, owns it in the resolved project, links it under `key`, returns `201`. On a collision (an `environment_vars` row already linked to `key`): `update` re-encrypts onto the existing item **in place** if it is exclusive (non-global, linked only here, owned only by this project) and returns `200`; if the existing item is shared instead, returns `409 SHARED_ITEM_CONFLICT`. `replace` always creates a new item and repoints the link (`201`), deleting the superseded item only if it is now unreachable (unlinked and non-global) — a shared item survives, still referenced elsewhere. `error` returns `409 KEY_EXISTS` on any collision. Create-or-update-and-link is one SQLite transaction — no interleaving leaves an item owned but unlinked. Caller-supplied `isGlobal` is ignored on both `Created` and `Updated` outcomes — the response reports the value actually persisted (always `false` on this path). Encrypts with AES-GCM before storing | | GET | /items/:id | token | Get single item metadata (redacted). **Unscoped** — reachable by id regardless of project/environment (scope is a display filter, not an access boundary; see Notes) | | PUT | /items/:id | token | Update item. Merges — omitted fields keep existing values including secret fields. Unscoped, same as GET /items/:id | | DELETE | /items/:id | token | Delete item. Returns 204. Unscoped | @@ -20,12 +20,12 @@ Authentication: Header `X-Vault-Token` containing either a session token (from P | POST | /categories | token | Create category. Validates name (req, max 100) and color (req). Generates random hex cid | | PUT | /categories/:id | token | Update category fields. Passing `description: ""` clears it | | DELETE | /categories/:id | token | Delete category. Returns 204 | -| GET | /commands | token | List items of type "command" with extracted `{{VAR}}` placeholders, **scoped** (same query params as GET /items) — limited to commands linked in the resolved environment | +| GET | /commands | token | List items of type "command" with extracted `{{VAR}}` placeholders, **scoped** (same query params as GET /items). **Discovery endpoint** — same `include_global` contract as GET /items (default `true`); each command carries `isGlobal` and `linked` | | GET | /commands/:id | token | Get single command with placeholders. Unscoped | | GET | /settings | token | Get auto_lock_timeout (minutes) and hotkey. Unscoped — settings are global | | PUT | /settings | token | Update auto_lock_timeout and/or hotkey | -| POST | /fill | token | Fill a .env template with real values, **scoped** (same query params as GET /items). Matches template keys against the resolved environment's `environment_vars.key` (not a vault-wide name search). A template key not found in scope has its **original line preserved unchanged** (not blanked) and is reported as a warning. `output_path` given: writes there via the RAII `TempEnvFile` guard, returns stats only — no secret in response. No `output_path` but `output_dir` given: writes `{output_dir}/.env.`. Neither: returns filled content inline. Body accepts `overwrite` (default `false`) — see **Write-target gating** below | -| POST | /environments/:id/example | token | Generate a placeholder-only env file/content for the environment (`environment_id` in URL path, same convention as `/environments/:id/inject`) — `KEY=` for every linked var key, values always empty, explicitly safe to commit. Never decrypts or reads item values. Body `{output_path?, output_dir?, overwrite?}`: `output_path` writes there; `output_dir` writes `{output_dir}/.env.example.`; neither returns `{content, keys}` inline. 404 if the environment doesn't resolve. Same `overwrite` gating as `/fill` — see below | +| POST | /fill | token | Fill a .env template with real values, **scoped** (same query params as GET /items). Matches template keys against the resolved environment's `environment_vars.key` (not a vault-wide name search). A template key not found in scope has its **original line preserved unchanged** (not blanked) and is reported as a warning. `output_path` given: writes there via the RAII `TempEnvFile` guard, returns stats only — no secret in response. No `output_path` but `output_dir` given: the environment-name-derived filename is resolved via `fsguard::resolve_within` (issue #7) before any decryption happens — `422 PATH_NOT_CONTAINED` if it can't stay inside `output_dir`, `path` in the response is the resolved (post-canonicalization) path. Neither: returns filled content inline. Body accepts `overwrite` (default `false`) — see **Write-target gating** below | +| POST | /environments/:id/example | token | Generate a placeholder-only env file/content for the environment (`environment_id` in URL path, same convention as `/environments/:id/inject`) — `KEY=` for every linked var key, values always empty, explicitly safe to commit. Never decrypts or reads item values. Body `{output_path?, output_dir?, overwrite?}`: `output_path` writes there; `output_dir` writes to the environment-name-derived filename, contained within `output_dir` via `fsguard::resolve_within` (issue #7, same as `/fill`) — `422 PATH_NOT_CONTAINED` on escape; neither returns `{content, keys}` inline. 404 if the environment doesn't resolve. Same `overwrite` gating as `/fill` — see below | | POST | /share/listen | token | Start LAN share session as sender, **scoped** (same query params as GET /items). Every id in `items` must already be linked into the resolved environment or the call 422s. Registers mDNS, returns `pairing_code` | | POST | /share/connect | token | Connect as receiver using pairing_code, **scoped** (same query params as GET /items). On successful transfer, received items are owned by the resolved project and linked into the resolved environment under the sender's item names — **except** where that name collides with a key already linked in the target environment, in which case the item is still imported/owned but the existing link is left untouched and the collision is reported (see `/share/status`'s `skipped_keys`, and Notes). Returns ECDH fingerprint | | POST | /share/confirm | token | Confirm (or reject) fingerprint. Both sides must call this | @@ -34,16 +34,79 @@ Authentication: Header `X-Vault-Token` containing either a session token (from P | POST | /share/export | token | Export items as AES-256-GCM encrypted `.vault` file. Returns passphrase in response. Unscoped (operates on item IDs directly) | | POST | /share/import | token | Import from `.vault` file using passphrase, **scoped** (same query params as GET /items). Imported items are owned by the resolved project and linked into the resolved environment, with the same collision-skip behavior as `/share/connect`. (The Tauri GUI command for this import path still passes no scope — items land ownerless/unlinked from the GUI, same pre-existing gap as before, not addressed this pass) | | GET | /projects | token | List all projects with their typed environments (name, template, paths, variable count) | -| POST | /projects | token | Create or update project. Returns project ID (upsert by id=0 for creation). `name` is unique case-insensitively at the DB level — creating with a name that already exists (any case) returns 409 CONFLICT instead of creating a duplicate | +| POST | /projects | token | Create or update project. Returns project ID (upsert by id=0 for creation). `name` is unique case-insensitively at the DB level — creating with a name that already exists (any case) returns 409 CONFLICT instead of creating a duplicate. `name` must also pass the filesystem-hostile deny-list (issue #7: no separators, control characters, NTFS-hostile characters, reserved device names, leading/trailing dot or whitespace; 128 chars max) — 422 `VALIDATION_ERROR` otherwise | | DELETE | /projects/:id | token | Delete project and all its environments. Returns impact summary | | GET | /projects/:id/preview-delete | token | Show what will be deleted (impact preview) without performing the deletion | -| POST | /environments | token | Create or update environment within a project. `projectId` is now required and validated to reference an existing project — 422 if missing/invalid. Returns environment ID (upsert by id=0 for creation) | +| POST | /environments | token | Create or update environment within a project. `projectId` is now required and validated to reference an existing project — 422 if missing/invalid. `name` must match `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$` (issue #7) — 422 `VALIDATION_ERROR` otherwise. Returns environment ID (upsert by id=0 for creation) | | DELETE | /environments/:id | token | Delete a single environment. Returns 204 | | POST | /environments/:id/inject | token | Inject environment's variables into its configured .env path(s). Takes a JSON body `{output_path?, output_dir?, overwrite?}` (previously bodyless — an empty `{}` body preserves the old behavior). `output_path`, if given, is added to (not a replacement for) the environment's configured `paths[]` — all get written. If `paths[]` is empty and no `output_path`, falls back to `{output_dir}/.env.`. Returns paths written, keys injected, `unmanagedPaths` (configured paths that were unmanaged and got written through anyway — see below) and `backups` (`.bak` paths created). Same `overwrite` gating as `/fill`, but only for `output_path`/`output_dir` — see below | | POST | /relay/send | token | Encrypt selected items with Argon2id-derived key and upload to Supabase relay. Returns code + passphrase. Requires relay_supabase_url and relay_supabase_anon_key in settings | | POST | /relay/receive | token | Download from Supabase relay, decrypt with key+passphrase, import items, **scoped** (same query params as GET /items). Imported items are owned by the resolved project and linked into the resolved environment, with the same collision-skip behavior as `/share/connect`. Burns after read (best-effort delete) | -| POST | /workspaces/:id/relay/send | token | Share complete workspace (definition + all decrypted referenced secrets) via relay. Returns code + passphrase. Legacy, workspace-table-backed, out of scope for the projects/environments migration | -| POST | /workspaces/relay/receive | token | Receive shared workspace from relay. Recreates secrets and rebuilds workspace with variables re-linked. Legacy, same as above — items imported this way are NOT linked into any project/environment and are invisible to the scoped endpoints above | +| POST | /projects/:id/relay/send | token | Share a whole project via relay: structure (environment names, `isDefault`) plus decrypted values for the selected `environment_ids`, deduped by item across environments. Returns `{code, passphrase, project, environment_count, item_count}`. Requires relay_supabase_url and relay_supabase_anon_key in settings | +| POST | /projects/relay/receive | token | Receive a shared project from relay. Always creates a **new** project (never merges) — a case-insensitive name collision returns `409 CONFLICT`; retry with `project_name_override` in the body. Received items are owned by the new project only (`isGlobal: false`), never linked into any pre-existing project. Returns `{project, environments, item_count}`. Burns after read (best-effort delete) | +| GET | /maintenance/orphans | token | Read-only, redacted list of items with zero `environment_vars` references and `isGlobal:false` (unreachable from any list/GUI surface). No REST prune endpoint by design — a static MCP token must not be able to bulk-delete vault rows; pruning is Tauri-command-only (`vault_prune_orphan_items`), behind the GUI's confirmation, same as every other destructive vault operation | + +### Examples + +`curl` examples against the local server. `-k` is required — the certificate +is self-signed (see Notes below). Replace `$TOKEN` with a session token from +`/unlock` or the static MCP token from Settings. + +```bash +# Unlock — returns a session token with a configurable TTL +curl -sk -X POST https://127.0.0.1:47821/unlock \ + -H 'Content-Type: application/json' \ + -d '{"master_password": "your-master-password"}' + +# List items — scoped by environment_id +curl -sk https://127.0.0.1:47821/items?environment_id=1 \ + -H "X-Vault-Token: $TOKEN" + +# List items — scoped by project + environment names (case-insensitive) +curl -sk 'https://127.0.0.1:47821/items?project=demo&environment=production' \ + -H "X-Vault-Token: $TOKEN" + +# Create an item, linked into an environment as DB_HOST +curl -sk -X POST 'https://127.0.0.1:47821/items?environment_id=1' \ + -H "X-Vault-Token: $TOKEN" -H 'Content-Type: application/json' \ + -d '{"type": "secret", "name": "DB_HOST", "value": "localhost", "key": "DB_HOST"}' + +# Reveal a plaintext value — requires explicit confirm +curl -sk -X POST https://127.0.0.1:47821/items/1/reveal \ + -H "X-Vault-Token: $TOKEN" -H 'Content-Type: application/json' \ + -d '{"confirm": true}' + +# Fill a .env template inline, scoped by project + environment +curl -sk -X POST 'https://127.0.0.1:47821/fill?project=demo&environment=production' \ + -H "X-Vault-Token: $TOKEN" -H 'Content-Type: application/json' \ + -d '{"template": "DB_HOST=\nPORT=3000\n"}' + +# List all projects with their nested environments +curl -sk https://127.0.0.1:47821/projects -H "X-Vault-Token: $TOKEN" + +# Inject an environment's variables into its configured .env path(s) +curl -sk -X POST https://127.0.0.1:47821/environments/1/inject \ + -H "X-Vault-Token: $TOKEN" -H 'Content-Type: application/json' -d '{}' +``` + +The server is HTTPS-only on `127.0.0.1:47821` with a self-signed certificate +generated on first launch (`tls::ensure_tls_config`) — clients must either +pass `-k`/`--insecure` (as above) or trust that certificate explicitly. + +**On the retired Postman collection**: `src-tauri/tests/crypt-env-api.postman_collection.json` +was deleted (tech-debt issue #11) — it asserted a `GET /health` field that no +longer exists and none of its 15 requests carried the (now mandatory) +project/environment scope, so every one of them 422'd. Nothing in CI ever +executed it, so it silently drifted out of date across a whole schema +migration. This reference section plus the `api::tests::*` suite (executed +on every push/PR — see `.github/workflows/test.yml`) are the replacement: +one documents the contract, the other proves it. A Postman collection may +return only alongside a test that replays it through the same router and +fails the build on drift — see the plan doc for the full reasoning. + +### Global items and scope + +**Discovery surfaces union globals; materialization surfaces never do; `linked` is the discriminator.** `GET /items` and `GET /commands` accept `include_global=true|false|only` (default `true`): `true` returns (items linked in the resolved environment) ∪ (all `isGlobal:true` items), deduplicated by id; `false` restricts to exactly what's linked — byte-for-byte what `/fill`/`/inject`/`/environments/:id/example` will materialize; `only` returns just the global set regardless of linkage (the REST equivalent of the GUI's Global Secrets screen), still requiring a valid scope. An invalid value 422s with `include_global` named in the message. Every returned item/command carries `isGlobal` (is it marked reusable) and `linked` (is it actually linked into the queried environment) so a caller can always tell "exists and reusable" apart from "will be written by fill/inject". `POST /fill`, `POST /environments/:id/inject`, `POST /environments/:id/example`, and `POST /share/listen` are unaffected by `include_global` — they resolve strictly through `environment_vars`, so linking a global into an environment remains a deliberate, explicit act. ### Examples @@ -119,6 +182,8 @@ Session token design uses a single `token_expires` slot (Instant monotonic), all CORS guard accepts `Origin: null`, correctly matching local file:// and Tauri webviews, but also matches any sandboxed iframe — minimal practical impact but violates defense-in-depth. +`PRAGMA secure_delete=ON` (issue #9): SQLite now zeroes freed page content on every `DELETE`/overwriting `UPDATE`, not just unlinking it — covers `delete_item`, `delete_project`'s cascades, the `POST /items?on_conflict=replace` superseded-item delete, and the orphan prune path uniformly. This changes on-disk behavior for *all* deletes going forward (connection-level setting, no on-disk format change, safe to remove at any time); rows deleted before this pragma was added are not retroactively scrubbed. + Projects/environments model replaces the old workspaces. A Project contains multiple typed Environments, each with its own paths and variables. Environment variables are real FK-based links (`environment_vars.item_id` → a vault item), not name-search — an item is only "in" an environment if explicitly linked via `POST /items` (with `key`), `POST /environments` (saving the var list), or one of the import paths above. Deleting a project cascades to delete all its environments via foreign key constraints. `/projects/:id/preview-delete` returns the impact without executing the deletion — allows clients to show the user what will be removed before confirming. @@ -129,7 +194,13 @@ Projects/environments model replaces the old workspaces. A Project contains mult `projects.name` has a case-insensitive UNIQUE index (`idx_projects_name_nocase`) — duplicate-name creation now returns 409 instead of silently succeeding. `environments.name` is only unique per-project under SQLite's default (case-sensitive) collation — two environments in the same project differing only by case (e.g. `Production`/`production`) can still coexist, and name-pair resolution (case-insensitive, picks the lowest-id match) will silently prefer one over the other with no ambiguity error. Known limitation, not fixed. -**Known, deferred issues** (found in review, not fixed in this pass): (1) a crafted environment `name` (only validated non-empty) combined with `output_dir` on `/fill`, `/environments/:id/inject`, or `/environments/:id/example` can path-traverse outside the intended directory, because `create_dir_all` on the joined path materializes the intermediate component that makes `..` segments resolve — reachable by anything holding the static MCP token via `POST /environments` (tracked separately; the write-target gating described above governs what may be done to a resolved path, not whether its construction is contained). (2) `POST /items` on a key that already exists in the environment creates a new item row and repoints the link, orphaning (not deleting) the previous item — repeated `add`-equivalent calls grow the vault unboundedly and "rotating" a secret this way doesn't actually remove the old value. (3) `PUT /items/:id`'s "merge" behavior only applies to the `Option` fields on `VaultItem` — `type` has no `#[serde(default)]`, so a client omitting it entirely gets a 422 from axum's `Json` extractor before the merge logic (or `validate_update`) ever runs; every partial update must still resend `type`. Found and pinned by `api::tests::items::update_item_partial_update_preserves_other_fields` (issue #11), not changed there since that's a behavior fix out of scope for a test-only PR. +**Environment/project name validation (issue #7, fixed):** `environment.name` must match `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$` (1-64 chars, starts with a letter or digit, no trailing `.` or `-`) — enforced in `project::save_environment` before the row is ever persisted, so it applies identically to `POST /environments`, the Tauri `environment_save` command, an imported `.cryptenv-proj` template, and the CLI. `project.name` gets a laxer deny-list instead (rejects separators, control characters, NTFS-hostile characters, reserved device names, leading/trailing dot or whitespace; allows spaces and non-ASCII letters; 128 chars max), enforced the same way in `project::save_project`. Rejection is `422 VALIDATION_ERROR`. Existing rows created before this validation shipped are **not** migrated or rejected at read time — they keep working (see the belt-and-braces containment below) and only get corrected the next time the row is edited. A second, independent layer (`fsguard::resolve_within`) additionally guarantees that no write derived from an environment name — validated or legacy — can land outside the caller-supplied `output_dir` on `/fill`, `/environments/:id/example`, or `project::inject_environment`'s default-filename branch; a name that somehow escapes both is a bug in this layer, not an accepted risk. + +**Known, deferred issues** (found in review, not fixed in this pass): (1) `PUT /items/:id`'s "merge" behavior only applies to the `Option` fields on `VaultItem` — `type` has no `#[serde(default)]`, so a client omitting it entirely gets a 422 from axum's `Json` extractor before the merge logic (or `validate_update`) ever runs; every partial update must still resend `type`. Found and pinned by `api::tests::items::update_item_partial_update_preserves_other_fields` (issue #11), not changed there since that's a behavior fix out of scope for a test-only PR. (2) `GET`/`PUT`/`DELETE /items/:id` and `POST /items/:id/reveal` perform no project/environment scope check — any valid token can reach any item by id. Scope is a display filter on the *list* endpoints, not an access boundary; issue #13 changes list visibility only and does not close this. Tracked separately. + +`project::save_environment` → `set_environment_vars` still deletes and re-inserts an environment's whole var set on save, so removing a variable in the GUI unlinks its item without deleting it (same orphan shape `POST /items` used to produce, from a different route). Deliberately not fixed here — whether unlinking a variable should also destroy its secret is a product decision; the orphan report below is the intended surface for it. `POST /share/import`, `POST /relay/receive`, and the GUI backup-import path can also create unlinked items and are equally not orphan-cleanup targets (see the prune predicate note below). + +**Orphan items** (unlinked, non-global vault rows — issue #9): every `environment_vars` key collision on `POST /items` used to leave the previously-linked item behind: still in `items`, still owned, still fully readable via `/items/:id` and `/items/:id/reveal`, just invisible to scoped listings. Fixed — see the `POST /items` row above for the new `on_conflict` semantics. `GET /maintenance/orphans` (redacted, read-only) and the Tauri commands `vault_list_orphan_items` (redacted list) / `vault_prune_orphan_items(ids)` (delete, GUI-only, typed confirmation) surface and clean up rows left behind by data written *before* this fix, or by the GUI/import routes noted above. This reference doesn't otherwise catalogue Tauri `invoke()` commands (GUI-internal, not a network-facing surface); these two are called out here because they are the only user-facing remediation for orphans and have no REST equivalent for the delete half. The underlying query (`items` with no `environment_vars` row and `isGlobal:false`) intentionally excludes global items — they retain a reachable surface via Global Secrets even when unlinked, so pruning them would delete something still visible elsewhere. There is deliberately no startup migration that prunes automatically: the same predicate matches freshly-received, not-yet-linked share/relay/backup imports, so an automatic sweep on unlock could delete something the user hasn't seen yet. --- @@ -166,10 +237,10 @@ Every command that reads or writes vault items scoped to a project (`add`, `fill | `doctor` | — | Check app health, vault lock state, token files, version, and validate `crypt-env.json` if present | | `fill` | `[PATH] [--project] [--env] [--force/-f]` | Fill a .env template with vault secrets from the resolved environment, via `POST /fill`. No PATH: looks for `.env.example` then `.env` in cwd; if neither exists, generates a fresh `.env` from the environment's own variable keys (inverse of `add`). `--force`/`-f` sends `overwrite: true` — required the first time the target already exists and wasn't created by crypt-env, otherwise the command exits non-zero with the server's 409 message on stderr. No interactive prompt (CI/pipe-safe) | | `inject` | `NAME [--shell TYPE] [--project] [--env]` | Prints shell assignment to stdout (safe for eval). Supported: pwsh, bash, zsh, sh. Prints verify hint to stderr | -| `list` | `[--project] [--env]` | List saved commands in a table | +| `list` | `[--project] [--env] [--scope-globals with\|without\|only]` | List saved commands in a table, with a SCOPE column (`linked`/`global`/`global+linked`). `--scope-globals` (default `with`) controls whether unlinked global commands are included | | `exec` | `NAME [ARGS] [--project] [--env]` | Execute a saved command by name | | `memory` | — | Save a command string interactively | -| `search` | `QUERY [--project] [--env]` | Search items by name/title within scope. Prints table of ID, TYPE, NAME, CATEGORIES. No values shown | +| `search` | `QUERY [--project] [--env] [--scope-globals with\|without\|only]` | Search items by name/title within scope. Prints table of ID, TYPE, NAME, SCOPE (`linked`/`global`/`global+linked`), CATEGORIES. `--scope-globals` (default `with`) controls whether unlinked global items are included. No values shown | | `set` | `NAME [--project] [--env]` | Print export/env assignment for a secret (stdout) | | `cmd` | `list/info/run [--project] [--env]` | Manage saved commands (list, get info, run) | | `share send` | `ITEM_IDS... [--project] [--env]` | Start LAN share as sender. Items must already be linked into the resolved environment. Polls for peer, shows fingerprint, prompts confirmation | @@ -207,14 +278,16 @@ Every command that reads or writes vault items scoped to a project (`add`, `fill `project inject` resolves environment by ID or by project+environment names (both case-insensitive). The environment's variables are matched via `environment_vars.item_id` (a real FK to the vault item), not by name search — the "matched by name" behavior only applies to legacy pre-migration rows still carrying a `literal` value with no `item_id`. Missing items appear as warnings but do not abort the injection. This subcommand (and `project delete-env`) still use `resolve_environment_id`/id-based lookups client-side and were intentionally left untouched by the project/environment scoping work — they were already project-scoped by construction. -`add` on a key that already exists in the resolved environment creates a new item row and repoints the environment-var link to it, rather than updating the existing item in place — the superseded item is orphaned (still in the vault, still decryptable via `/items/:id`, included in exports/backups) rather than deleted. Not fixed this pass. +`add` on a key that already exists in the resolved environment now updates the existing item **in place** (`POST /items` default `?on_conflict=update`) — the previous value is destroyed, matching the "Update all conflicting keys?" prompt's wording. Confirming (or `--force`) sends the default `update` mode. Declining no longer silently drops the key client-side — it is still sent, but with `?on_conflict=error`, so a genuine remaining collision is reported as `Conflict for '' [KEY_EXISTS]: ...` rather than the CLI pretending nothing was asked for it. If the existing item is shared (global, linked in another environment, or multi-owned), the update is rejected with `SHARED_ITEM_CONFLICT` instead of silently rewriting a value another environment/project also sees; the CLI prints the key and reason, not a bare `HTTP 409`. -A crafted environment name (created via the GUI or with the static MCP token — CLI-driven `crypt-env.json`/cwd-derived names can't produce this) combined with `--project`/`--env` resolving to it can path-traverse `fill`'s/`project inject`'s `output_dir`-derived path outside the intended directory. Not fixed this pass. +A crafted environment name (created via the GUI or with the static MCP token — CLI-driven `crypt-env.json`/cwd-derived names can't produce this) combined with `--project`/`--env` resolving to it could previously path-traverse `fill`'s/`project inject`'s `output_dir`-derived path outside the intended directory — fixed by issue #7 (see the "Environment/project name validation" note above): the name charset is now enforced on write, and `fsguard::resolve_within` independently contains any legacy row that predates the check. `project list` shows all projects with nested environment details. Projects with no environments display "(none)" for environment and var count. `doctor` no longer reports vault item count — `GET /health` stopped returning `item_count` (it leaked vault size to unauthenticated callers). +`cmd`/`exec` resolve a command by name against `GET /commands`, which now defaults to unioning in unlinked global commands (issue #13). When a linked command and a global command share the same name, the linked one wins and a one-line warning naming the shadowed global's id is printed to stderr — `crypt-env cmd`/`crypt-env exec` do not take `--scope-globals` themselves (they always resolve with the default union so a global command remains runnable from any project). + --- ## TUI @@ -276,16 +349,16 @@ Authentication: Automatic — MCP server reads the REST API session token from d | Tool | Required | Optional | Description | |------|----------|----------|-------------| -| `crypt_env_list_items` | `environment_id` or (`project`+`environment`) | `type`, `category` | List item metadata (no values), scoped to a project+environment (required — the underlying `GET /items` now enforces it). Filter by type or category on top | +| `crypt_env_list_items` | `environment_id` or (`project`+`environment`) | `type`, `category`, `include_global` | List item metadata (no values), scoped to a project+environment (required — the underlying `GET /items` now enforces it). `include_global` (`true`\|`false`\|`only`, default `true`) also lists reusable global secrets not yet linked into this environment — these appear with `linked: false` and will NOT be written by generate/inject/fill until linked. Filter by type or category on top | | `crypt_env_get_item` | `id` | — | Get single item metadata (no value). Unscoped — `GET /items/:id` was not changed by this migration, reachable by id regardless of project | -| `crypt_env_search_items` | `query`, `environment_id` or (`project`+`environment`) | — | Search items by name within scope. Returns metadata only | -| `crypt_env_add_item` | `type`, `name`, `environment_id` or (`project`+`environment`) | `value`, `category`, `notes`, `url`, `username`, `key` | Add item to vault, owned by the resolved project and linked into the resolved environment under `key` (defaults to `name`). Value passes through MCP → REST in plaintext | +| `crypt_env_search_items` | `query`, `environment_id` or (`project`+`environment`) | `include_global` | Search items by name within scope. Same `include_global` contract as `crypt_env_list_items`. Returns metadata only | +| `crypt_env_add_item` | `type`, `name`, `environment_id` or (`project`+`environment`) | `value`, `category`, `notes`, `url`, `username`, `key`, `on_conflict` | Add item to vault, owned by the resolved project and linked into the resolved environment under `key` (defaults to `name`). If `key` already exists in the target environment, the existing item is updated in place (`on_conflict` default `update`) — its previous value is destroyed. If that item is shared with other environments/projects (or is global), the call fails with `SHARED_ITEM_CONFLICT` instead of silently changing it elsewhere; retry with `on_conflict: "replace"` to create a new item and repoint just this link, or use `crypt_env_update_item` to change the shared value everywhere. `on_conflict: "error"` fails on any existing key. Value passes through MCP → REST in plaintext | | `crypt_env_update_item` | `id` | `name`, `value`, `url`, `username`, `password`, `title`, `description`, `notes`, `content`, `command`, `shell`, `categories` | Update item. Omitted fields keep existing values server-side. Unscoped — `PUT /items/:id` was not changed by this migration | | `crypt_env_delete_item` | `id` | — | Permanently delete item. Unscoped — `DELETE /items/:id` was not changed by this migration | | `crypt_env_generate_env` | `keys`, `environment_id` or (`project`+`environment`) | — | Write .env file to temp dir with real values for given key names, looked up by item name within scope (see Notes for the name-vs-key mismatch vs `crypt_env_fill_env`). Returns path + count. Values never in response. Cleans up previous temp file on next call | | `crypt_env_inject_env` | `key`, `environment_id` or (`project`+`environment`) | — | Inject one secret as env var into the MCP process via `std::env::set_var`. Does not return value | | `crypt_env_fill_env` | `template` | `output_path`, `output_dir`, `overwrite`, `environment_id` or (`project`+`environment`) | Fill a template with vault secrets from the resolved environment, matched by `environment_vars.key`. `output_path` given: writes there. No `output_path` but `output_dir`: writes `{output_dir}/.env.`. Neither: **filled content returned inline in the tool response** — a value-exposure path that didn't exist when `output_path` was required. Refuses to overwrite a file it didn't create unless `overwrite: true` — see Notes | -| `crypt_env_import_env_file` | `path`, `environment_id` or (`project`+`environment`) | `category`, `overwrite` | Read .env file from disk, parse KEY=value pairs, import each as a vault item owned by the resolved project and linked into the resolved environment. Values never in MCP response | +| `crypt_env_import_env_file` | `path`, `environment_id` or (`project`+`environment`) | `category`, `overwrite` | Read .env file from disk, parse KEY=value pairs, import each as a vault item owned by the resolved project and linked into the resolved environment. Existing item found by name: `overwrite:true` updates it in place (`PUT`, destroying its previous value), `overwrite:false` (default) skips it. Not found by name but the environment key still collides (renamed item, or a race): the create call maps `overwrite:true` → `on_conflict=update` and `overwrite:false` → `on_conflict=error`, and a resulting `409` is folded into the same `skipped_existing` report bucket rather than `errors` — this is what stops bulk import from mass-producing orphans (issue #9). Values never in MCP response | | `crypt_env_update_settings` | — | `auto_lock_timeout`, `hotkey` | Update vault settings | | `crypt_env_doctor` | — | — | Health check: app status, lock state, token config. No longer reports item count — `GET /health` stopped returning `item_count` | | `crypt_env_list_commands` | `environment_id` or (`project`+`environment`) | — | List saved commands with placeholders, scoped to a project+environment | @@ -308,8 +381,6 @@ Authentication: Automatic — MCP server reads the REST API session token from d | `crypt_env_inject_env_by_name` | `project_path`, `environment` | `output_path` (unused, kept for compatibility) | Inject environment variables for a project directory and environment name. Matches by real environment name; if no matching environment is found, returns an error with next steps — the previous item-naming-convention fallback was removed (see Notes) | | `crypt_env_relay_send` | `item_ids` | — | Send via internet relay. Returns code + passphrase (show immediately, only once) | | `crypt_env_relay_receive` | `code`, `passphrase`, `environment_id` or (`project`+`environment`) | — | Receive via internet relay. Imported items are owned by the resolved project and linked into the resolved environment, same collision-skip behavior as `crypt_env_share_connect` | -| `crypt_env_share_workspace_send` | — | `id` or `name` | Share complete workspace (definition + all decrypted secrets) via relay. Returns code + passphrase | -| `crypt_env_share_workspace_receive` | `code`, `passphrase` | — | Receive shared workspace from relay. Recreates secrets and rebuilds workspace with variables re-linked | | `crypt_env_list_mcp_servers` | — | `scope` | List registered MCP servers from Claude config. Scope: global/project/all. Env values never returned | | `crypt_env_add_mcp_server` | `name`, `command` | `args`, `env`, `scope` | Add MCP server to Claude config. `env` stores KEY: "" placeholders — never real values | | `crypt_env_update_mcp_server` | `name` | `command`, `args`, `env`, `scope` | Merge-update existing MCP server entry | @@ -339,9 +410,9 @@ Authentication: Automatic — MCP server reads the REST API session token from d `crypt_env_inject_env_by_name` resolves by project directory + environment name. If a real environment is found, its paths are used. If not found, the tool now returns an error with next steps (pass an explicit `environment_id`, or `project`+`environment`) — the previous fallback to item-naming-convention matching (name prefix, category) was removed, since it relied on the now-scope-required `/items`/`/fill` endpoints in a way that could no longer work safely. `output_path` is accepted for backward compatibility but is currently unused. -Global items (`isGlobal: true`, not linked into the queried environment) are invisible to `crypt_env_list_items`, `crypt_env_search_items`, `crypt_env_generate_env`, and `crypt_env_inject_env` — there is currently no MCP tool that can discover a project's reusable global secrets; only items explicitly linked into the scoped environment are reachable. +Fixed (issue #13): `crypt_env_list_items` and `crypt_env_search_items` now default to `include_global=true`, unioning in reusable global items (`isGlobal: true`) that are not yet linked into the queried environment — each result carries `isGlobal` and `linked` so the agent can tell "exists and reusable" apart from "will actually be written". Pass `include_global=false` to see exactly the linked set, or `only` to see just the globals. The remaining gap is unchanged: `crypt_env_generate_env` and `crypt_env_inject_env` still resolve strictly by linkage (matching `/fill`'s/`/inject`'s materialization-only contract) — a global secret discovered via `crypt_env_list_items` still requires an explicit link into the environment (e.g. via `crypt_env_add_item`) before either of those tools can write it. -`crypt_env_share_workspace_send` and `crypt_env_share_workspace_receive` are retained for backward compatibility — they share complete workspaces (definition + decrypted secrets) via relay, not individual items. +Whole-project relay sharing (`POST /projects/:id/relay/send` / `/projects/relay/receive`) intentionally has **no MCP tool** — sending an entire project's decrypted secrets to a third-party relay from a single agent-callable tool, with no way for the agent to meaningfully obtain the sender's confirmation, is a materially different trust decision than the existing per-item `crypt_env_relay_send`/`_receive`. If ever wanted, it needs its own consent design as a separate change. MCP server is single-threaded with a blocking I/O loop (`stdin.lock().lines()`) — a slow or stalled request blocks all subsequent MCP tool calls and the LLM host may time out other concurrent requests. diff --git a/src-tauri/src/api/mod.rs b/src-tauri/src/api/mod.rs index 769d916..73a37fd 100644 --- a/src-tauri/src/api/mod.rs +++ b/src-tauri/src/api/mod.rs @@ -14,8 +14,11 @@ use tokio::sync::Mutex; use zeroize::Zeroizing; use crate::crypto; -use crate::db::{DbCategory, DbWorkspaceVar}; +// `DbWorkspaceVar` went unused when issue #4 replaced the workspace-relay +// handlers with the project-relay ones. +use crate::db::DbCategory; use crate::envfile; +use crate::fsguard; use crate::project::{self, EnvironmentInput, ProjectInput}; use crate::share::{ShareState, ShareSessionState}; use crate::share::relay; @@ -93,6 +96,19 @@ struct CommandDetail { placeholders: Vec, } +/// `/commands`-list-only wrapper adding the same `isGlobal`/`linked` +/// discriminators as `ScopedItem`, kept off the shared `CommandDetail` (used +/// unscoped by `GET /commands/:id`, which this change deliberately leaves +/// alone — see plan §3/§4). +#[derive(Serialize)] +struct ScopedCommand { + #[serde(flatten)] + detail: CommandDetail, + #[serde(rename = "isGlobal")] + is_global: bool, + linked: bool, +} + #[derive(Serialize)] struct RevealResponse { value: String, @@ -280,6 +296,120 @@ struct EnvScopeQuery { environment_id: Option, project: Option, environment: Option, + /// Only consumed by `handle_list_commands` — see `IncludeGlobal`. Present + /// here (rather than only on `ItemsQuery`) because `/commands` shares + /// this extractor; other handlers reusing `EnvScopeQuery` simply ignore + /// an unused query param, matching the existing per-handler duplication + /// style instead of refactoring the shared extractor in a bug-fix PR. + include_global: Option, +} + +/// Discovery-endpoint tri-state for whether globally-reusable, unlinked +/// items are unioned into the response. Materialization endpoints +/// (`/fill`, `/environments/:id/inject`, `/environments/:id/example`, +/// `/share/listen`) never consult this — they stay strictly linkage-based. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +enum IncludeGlobal { + With, + Without, + Only, +} + +impl IncludeGlobal { + /// `None` (param omitted) defaults to `With` — see plan §4.5: a + /// default-`false` fix is invisible to callers who don't know the + /// param exists, which is precisely the bug being fixed. + fn parse(raw: Option<&str>) -> Result { + match raw { + None | Some("true") | Some("with") => Ok(IncludeGlobal::With), + Some("false") | Some("without") => Ok(IncludeGlobal::Without), + Some("only") => Ok(IncludeGlobal::Only), + Some(_) => Err(err_validation( + "include_global", + "must be one of: true, false, only", + )), + } + } +} + +/// Union (or restriction) of `items` against the `linked` id set, per `mode`. +/// Pure function — no `ApiState`, no lock, no crypto — fully unit-testable +/// without a vault or an HTTP server. Stamps `linked` on every returned item. +fn scope_items(items: Vec, linked: &HashSet, mode: IncludeGlobal) -> Vec { + items + .into_iter() + .filter_map(|item| { + let is_linked = linked.contains(&item.id); + let is_global = item.is_global.unwrap_or(false); + let include = match mode { + IncludeGlobal::With => is_linked || is_global, + IncludeGlobal::Without => is_linked, + IncludeGlobal::Only => is_global, + }; + include.then(|| ScopedItem { linked: is_linked, item }) + }) + .collect() +} + +/// Applies the `/items` type/category/search filters over `ScopedItem`s. +/// `type_filter`/`cat_filter`/`search_filter` are expected pre-lowercased by +/// the caller, matching the pre-existing filter behaviour byte-for-byte. +fn filter_scoped_items( + items: Vec, + type_filter: Option<&str>, + cat_filter: Option<&str>, + search_filter: Option<&str>, +) -> Vec { + items + .into_iter() + .filter(|s| { + if let Some(t) = type_filter { + if s.item.item_type.to_lowercase() != t { + return false; + } + } + if let Some(cat) = cat_filter { + let found = s + .item + .categories + .iter() + .flatten() + .any(|c| c.to_lowercase() == cat); + if !found { + return false; + } + } + if let Some(q) = search_filter { + let name_match = s + .item + .name + .as_deref() + .map(|n| n.to_lowercase().contains(q)) + .unwrap_or(false); + let title_match = s + .item + .title + .as_deref() + .map(|t| t.to_lowercase().contains(q)) + .unwrap_or(false); + if !name_match && !title_match { + return false; + } + } + true + }) + .collect() +} + +/// API-response-only wrapper adding the `linked` discriminator on top of +/// `VaultItem`. MUST NOT be merged into `VaultItem` — that struct is what +/// gets AES-GCM encrypted (`vault::encrypt_item`), so a view-only field on +/// it risks being persisted into ciphertext by any round-trip write path. +#[derive(Serialize)] +struct ScopedItem { + #[serde(flatten)] + item: VaultItem, + linked: bool, } /// Resolves the environment for a scoped request. A missing/unmatched @@ -493,6 +623,8 @@ struct ItemsQuery { environment_id: Option, project: Option, environment: Option, + /// Tri-state `true|false|only`, default `true` — see `IncludeGlobal`. + include_global: Option, } /// The set of item ids linked into an environment's `environment_vars` — the @@ -528,6 +660,11 @@ async fn handle_list_items( }; let allowed_ids = environment_item_ids(&env); + let mode = match IncludeGlobal::parse(params.include_global.as_deref()) { + Ok(m) => m, + Err(resp) => return resp, + }; + let items = match decrypt_all_items(&state).await { Ok(i) => i, Err(StatusCode::FORBIDDEN) => { @@ -539,52 +676,24 @@ async fn handle_list_items( .into_response() } }; - let items = items.into_iter().filter(|item| allowed_ids.contains(&item.id)); + + // Union linked items with globals (per `mode`) before applying the + // type/category/search filters, so `search` also searches globals. + let scoped = scope_items(items, &allowed_ids, mode); let type_filter = params.item_type.as_deref().map(|s| s.to_lowercase()); let cat_filter = params.category.as_deref().map(|s| s.to_lowercase()); let search_filter = params.search.as_deref().map(|s| s.to_lowercase()); - let filtered: Vec = items - .into_iter() - .filter(|item| { - // Filtro por tipo - if let Some(ref t) = type_filter { - if item.item_type.to_lowercase() != *t { - return false; - } - } - // Filtro por categoría - if let Some(ref cat) = cat_filter { - let found = item - .categories - .iter() - .flatten() - .any(|c| c.to_lowercase() == *cat); - if !found { - return false; - } - } - // Filtro por búsqueda en nombre/título - if let Some(ref q) = search_filter { - let name_match = item - .name - .as_deref() - .map(|n| n.to_lowercase().contains(q.as_str())) - .unwrap_or(false); - let title_match = item - .title - .as_deref() - .map(|t| t.to_lowercase().contains(q.as_str())) - .unwrap_or(false); - if !name_match && !title_match { - return false; - } - } - true - }) - .map(redact_item) - .collect(); + let filtered: Vec = filter_scoped_items( + scoped, + type_filter.as_deref(), + cat_filter.as_deref(), + search_filter.as_deref(), + ) + .into_iter() + .map(|s| ScopedItem { item: redact_item(s.item), linked: s.linked }) + .collect(); (StatusCode::OK, Json(filtered)).into_response() } @@ -631,10 +740,30 @@ struct CreateItemBody { key: Option, } +/// Separate extractor (rather than folding `on_conflict` into `EnvScopeQuery`, +/// which every scoped GET/PUT/DELETE endpoint also uses) so that only +/// `POST /items` gives the parameter any meaning — everywhere else it would +/// be silently accepted and ignored, which is worse than a second struct. +#[derive(Deserialize)] +struct ConflictQuery { + on_conflict: Option, +} + +fn parse_link_mode(raw: Option<&str>) -> Result { + match raw { + None => Ok(crate::db::LinkMode::Update), + Some("update") => Ok(crate::db::LinkMode::Update), + Some("replace") => Ok(crate::db::LinkMode::Replace), + Some("error") => Ok(crate::db::LinkMode::Error), + Some(_) => Err(err_validation("on_conflict", "must be one of: update, replace, error")), + } +} + async fn handle_create_item( State(state): State>, headers: HeaderMap, Query(scope): Query, + Query(conflict): Query, Json(mut body): Json, ) -> impl IntoResponse { if let Err(code) = verify_token(&headers, &state).await { @@ -646,6 +775,11 @@ async fn handle_create_item( return err_json(code, msg, err_code).into_response(); } + let mode = match parse_link_mode(conflict.on_conflict.as_deref()) { + Ok(m) => m, + Err(resp) => return resp, + }; + let env = match resolve_scope( &state, scope.environment_id, @@ -677,7 +811,7 @@ async fn handle_create_item( body.item.created = now_ts_str(); } - let new_id = { + let outcome = { let vault = state.vault.lock().await; let key = match vault.key.as_ref() { Some(k) => k.clone(), @@ -687,27 +821,51 @@ async fn handle_create_item( } }; - // Create + own the item atomically (same primitive as the Tauri - // command `vault_create_project_item`), then link it into this - // environment's vars under `key_name`. - let new_id = match crate::vault::create_project_item(&vault.db, &key, &body.item, env.project_id).await { - Ok(id) => id, - Err(e) => { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") - .into_response() - } - }; - - if let Err(e) = vault.db.upsert_environment_var(env.id, &key_name, new_id).await { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR").into_response(); - } - - new_id + crate::vault::create_or_update_env_item( + &vault.db, + &key, + &body.item, + env.project_id, + env.id, + &key_name, + mode, + ) + .await }; - body.item.id = new_id; - body.item.is_global = Some(false); - (StatusCode::CREATED, Json(redact_item(body.item))).into_response() + match outcome { + Ok(crate::vault::UpsertOutcome::Created(item)) => { + (StatusCode::CREATED, Json(redact_item(item))).into_response() + } + Ok(crate::vault::UpsertOutcome::Updated(item)) => { + (StatusCode::OK, Json(redact_item(item))).into_response() + } + Ok(crate::vault::UpsertOutcome::Conflict { item_id, reason }) => match reason { + crate::vault::ConflictReason::Shared => err_json( + StatusCode::CONFLICT, + &format!( + "key '{key_name}' is linked to item {item_id}, which is shared (global, multi-linked, \ + or multi-owned). Use ?on_conflict=replace to repoint this link to a new copy, or \ + PUT /items/{item_id} to change the shared value everywhere." + ), + "SHARED_ITEM_CONFLICT", + ) + .into_response(), + crate::vault::ConflictReason::KeyExists => err_json( + StatusCode::CONFLICT, + &format!("key '{key_name}' already exists in this environment"), + "KEY_EXISTS", + ) + .into_response(), + crate::vault::ConflictReason::StateChanged => err_json( + StatusCode::CONFLICT, + &format!("key '{key_name}' changed concurrently, retry the request"), + "CONFLICT_RETRY", + ) + .into_response(), + }, + Err(e) => err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR").into_response(), + } } async fn handle_update_item( @@ -847,6 +1005,43 @@ async fn handle_delete_item( } } +/// Read-only, redacted report of items with no `environment_vars` reference +/// and `is_global = 0` (issue #9 §3.6). Deliberately no REST prune endpoint — +/// a static MCP token should not be able to bulk-delete vault rows; the +/// destructive half stays behind the GUI's confirmation +/// (`vault_prune_orphan_items`), matching every other destructive vault +/// operation. Lets `crypt-env doctor` surface a one-line orphan count. +async fn handle_list_orphans( + State(state): State>, + headers: HeaderMap, +) -> impl IntoResponse { + if let Err(code) = verify_token(&headers, &state).await { + let (msg, err_code) = match code { + StatusCode::UNAUTHORIZED => ("no autorizado", "UNAUTHORIZED"), + StatusCode::FORBIDDEN => ("bóveda bloqueada", "VAULT_LOCKED"), + _ => ("error interno", "INTERNAL_ERROR"), + }; + return err_json(code, msg, err_code).into_response(); + } + + let vault = state.vault.lock().await; + let key = match vault.key.as_ref() { + Some(k) => k.clone(), + None => { + return err_json(StatusCode::FORBIDDEN, "bóveda bloqueada", "VAULT_LOCKED") + .into_response() + } + }; + + match crate::vault::list_orphan_items(&vault.db, &key).await { + Ok(items) => { + let redacted: Vec = items.into_iter().map(redact_item).collect(); + (StatusCode::OK, Json(redacted)).into_response() + } + Err(e) => err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR").into_response(), + } +} + async fn handle_list_categories( State(state): State>, headers: HeaderMap, @@ -1050,6 +1245,11 @@ async fn handle_list_commands( }; let allowed_ids = environment_item_ids(&env); + let mode = match IncludeGlobal::parse(scope.include_global.as_deref()) { + Ok(m) => m, + Err(resp) => return resp, + }; + let items = match decrypt_all_items(&state).await { Ok(i) => i, Err(StatusCode::FORBIDDEN) => { @@ -1062,19 +1262,26 @@ async fn handle_list_commands( } }; - let commands: Vec = items + let commands: Vec = scope_items(items, &allowed_ids, mode) .into_iter() - .filter(|item| item.item_type == "command" && allowed_ids.contains(&item.id)) - .map(|item| { + .filter(|s| s.item.item_type == "command") + .map(|s| { + let is_global = s.item.is_global.unwrap_or(false); + let linked = s.linked; + let item = s.item; let template = item.command.as_deref().unwrap_or(""); let placeholders = extract_placeholders(template); - CommandDetail { - id: item.id, - name: item.name.unwrap_or_default(), - description: item.description, - shell: item.shell, - command: item.command, - placeholders, + ScopedCommand { + detail: CommandDetail { + id: item.id, + name: item.name.unwrap_or_default(), + description: item.description, + shell: item.shell, + command: item.command, + placeholders, + }, + is_global, + linked, } }) .collect(); @@ -1432,6 +1639,51 @@ async fn handle_fill( Err(resp) => return resp, }; + // Resolve and validate the write target *before* any decryption happens + // (issue #7, objective 3): on rejection, no plaintext has been produced + // for this request, on disk or in memory. + // + // `output_path` is exact caller intent for *this* request and is passed + // through verbatim — validating it is issue #8's territory (clobber / + // no-clobber), not this one. `output_dir` only ever decides the + // *filename* inside it, and that filename is derived from the + // environment's `name` — untrusted, persisted data — so it goes through + // `fsguard::resolve_within`, which guarantees the result cannot land + // outside the caller-supplied directory. + let write_target: Option = if let Some(out) = body.output_path.as_deref() { + Some(PathBuf::from(out)) + } else if let Some(dir) = body.output_dir.as_deref() { + match fsguard::resolve_within(dir, &format!(".env.{}", env.name)) { + Ok(p) => Some(p), + Err(fsguard::ContainmentError::BaseUnusable(msg)) => { + return err_json( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("cannot use output directory: {msg}"), + "INTERNAL_ERROR", + ) + .into_response(); + } + Err(e) => { + // Never echo the environment name or the resolved path (see + // plan §4/D5) — the id is enough to identify the row, and a + // rejection firing at all means a hostile name reached a + // sink and layer 2 caught it. + eprintln!( + "[api] /fill rejected: environment id {} — output_dir not contained ({e:?})", + env.id + ); + return err_json( + StatusCode::UNPROCESSABLE_ENTITY, + "output_dir: environment name does not resolve to a path contained within the requested directory", + "PATH_NOT_CONTAINED", + ) + .into_response(); + } + } + } else { + None + }; + let items = match decrypt_all_items(&state).await { Ok(i) => i, Err(StatusCode::FORBIDDEN) => { @@ -1510,33 +1762,29 @@ async fn handle_fill( filled.push('\n'); } - // Explicit output_path: write to exactly that file. No output_path but an - // output_dir: write to the default-filename-with-environment-suffix - // convention inside it. Neither: return content inline (unchanged). - let write_target = if let Some(out) = body.output_path { - Some(out) - } else if let Some(dir) = body.output_dir { - let dir = dir.trim_end_matches(['/', '\\']); - Some(format!("{dir}/.env.{}", env.name)) - } else { - None - }; - // When writing to disk: write via RAII guard, return stats only (no - // secret content in the response). + // secret content in the response). `write_target` was already resolved + // and validated above, before decryption. // // The guard zeros and deletes the file if any error occurs before persist(). // On success, persist() disarms the guard so the caller can consume the file. - if let Some(out) = write_target { - let path = std::path::PathBuf::from(&out); - if let Some(parent) = path.parent() { - if let Err(e) = std::fs::create_dir_all(parent) { - return err_json( - StatusCode::INTERNAL_SERVER_ERROR, - &format!("cannot create directory: {e}"), - "INTERNAL_ERROR", - ) - .into_response(); + if let Some(path) = write_target { + // Only the explicit `output_path` branch needs a directory created + // here: it is exact caller intent with no interpolated name in it. + // The `output_dir` branch's base was already created inside + // `fsguard::resolve_within` above, on the caller-supplied directory + // alone (issue #7, objective 4 — no `create_dir_all` ever sees a + // path with an interpolated name in it). + if body.output_path.is_some() { + if let Some(parent) = path.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + return err_json( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("cannot create directory: {e}"), + "INTERNAL_ERROR", + ) + .into_response(); + } } } @@ -2058,6 +2306,14 @@ async fn handle_save_project( err_json(StatusCode::CONFLICT, "a project with this name already exists", "CONFLICT") .into_response() } + // `project::save_project` runs `validate_project_name` as its first + // statement (issue #7's choke point) and prefixes its message with + // "name: " on failure — surface that as a caller error, not a + // server fault. Safe to echo: `validate_project_name` returns the + // rule that was broken and never the input `name` itself. + Err(e) if e.starts_with("name: ") => { + err_json(StatusCode::UNPROCESSABLE_ENTITY, &e, "VALIDATION_ERROR").into_response() + } // Never echo `e` here: on any other failure it may carry raw sqlx/SQL // text (table, column, index names), which CLAUDE.md forbids in an // API response. Log the detail server-side instead — never the @@ -2161,6 +2417,14 @@ async fn handle_save_environment( let status = if is_new { StatusCode::CREATED } else { StatusCode::OK }; (status, Json(serde_json::json!({ "id": id }))).into_response() } + // `project::save_environment` runs `validate_environment_name` as + // its first statement (issue #7's choke point — the same check the + // Tauri command, CLI, and an imported `.cryptenv-proj` template all + // go through) and prefixes its message with "name: " on failure. + // Safe to echo: the message names the rule, never the input `name`. + Err(e) if e.starts_with("name: ") => { + err_json(StatusCode::UNPROCESSABLE_ENTITY, &e, "VALIDATION_ERROR").into_response() + } // Same "conflict:" sentinel contract as `handle_save_project` — set // either by `db::upsert_environment`'s unique-index violation (ASCII // case) or `project::ensure_no_case_collision`'s app-level @@ -2331,26 +2595,55 @@ async fn handle_environment_example( // values are never read or decrypted here. let content = keys.iter().map(|k| format!("{k}=")).collect::>().join("\n") + "\n"; - let write_target = if let Some(p) = body.output_path { - Some(p) - } else if let Some(dir) = body.output_dir { - let dir = dir.trim_end_matches(['/', '\\']); - Some(format!("{dir}/.env.example.{}", env.name)) - } else { - None - }; - - if let Some(out) = write_target { - let path = std::path::PathBuf::from(&out); - if let Some(parent) = path.parent() { - if let Err(e) = std::fs::create_dir_all(parent) { + // Same containment split as `/fill` (issue #7): `output_path` is exact + // caller intent, passed through verbatim; `output_dir` only picks the + // filename inside it, and that filename is derived from the untrusted, + // persisted environment `name`, so it goes through `fsguard`. Content + // here is placeholder keys only (never decrypted), so there is no + // decrypt-ordering concern like `/fill`'s objective 3 — but the code + // shape is kept the same so the two sinks stay diff-comparable. + let write_target: Option = if let Some(p) = body.output_path.as_deref() { + Some(PathBuf::from(p)) + } else if let Some(dir) = body.output_dir.as_deref() { + match fsguard::resolve_within(dir, &format!(".env.example.{}", env.name)) { + Ok(p) => Some(p), + Err(fsguard::ContainmentError::BaseUnusable(msg)) => { return err_json( StatusCode::INTERNAL_SERVER_ERROR, - &format!("cannot create directory: {e}"), + &format!("cannot use output directory: {msg}"), "INTERNAL_ERROR", ) .into_response(); } + Err(e) => { + eprintln!( + "[api] /environments/{{id}}/example rejected: environment id {} — output_dir not contained ({e:?})", + env.id + ); + return err_json( + StatusCode::UNPROCESSABLE_ENTITY, + "output_dir: environment name does not resolve to a path contained within the requested directory", + "PATH_NOT_CONTAINED", + ) + .into_response(); + } + } + } else { + None + }; + + if let Some(path) = write_target { + if body.output_path.is_some() { + if let Some(parent) = path.parent() { + if let Err(e) = std::fs::create_dir_all(parent) { + return err_json( + StatusCode::INTERNAL_SERVER_ERROR, + &format!("cannot create directory: {e}"), + "INTERNAL_ERROR", + ) + .into_response(); + } + } } // No RAII zero-wipe needed here — the content holds no secret value, // only key names. It still gets the same existence gate and marker @@ -2359,6 +2652,10 @@ async fn handle_environment_example( // `FileMode::Inherit` (not `Private0600`) because this file exists // to be committed and shared — forcing owner-only permissions on a // `.env.example` would be surprising on a shared build machine. + // + // `path` is already containment-checked above (issue #7): the + // `output_dir` branch routes through `fsguard::resolve_within`, so + // the reported path is the post-canonicalization one. let marker = build_marker(&state, &env).await; let opts = envfile::WriteOptions { overwrite: body.overwrite, mode: envfile::FileMode::Inherit }; let committed = match envfile::commit(&path, &content, &marker, &opts) { @@ -2366,7 +2663,8 @@ async fn handle_environment_example( Err(e) => return err_envfile(e), }; let backup = committed.backup.map(|p| p.to_string_lossy().into_owned()); - return (StatusCode::OK, Json(ExampleResponse { content: None, path: Some(out), keys, backup })).into_response(); + let resolved = path.to_string_lossy().into_owned(); + return (StatusCode::OK, Json(ExampleResponse { content: None, path: Some(resolved), keys, backup })).into_response(); } (StatusCode::OK, Json(ExampleResponse { content: Some(content), path: None, keys, backup: None })).into_response() @@ -2736,21 +3034,37 @@ async fn handle_relay_receive( .into_response() } -// ─── Complete-workspace relay handlers ──────────────────────────────────────── +// ─── Project relay handlers (issue #4 — share a whole project) ─────────────── +// Replaces the deleted `/workspaces/*/relay/*` pair: same relay transport, +// but the payload is a `ProjectBundle` (structure + values for N +// environments at once) instead of a single flat item list or a +// frozen-table-backed workspace. See `project::relay` for the orchestration +// (build/receive) and `share::relay::ProjectBundle` for the wire format. + +#[derive(serde::Deserialize)] +struct ProjectRelaySendBody { + environment_ids: Vec, +} #[derive(serde::Serialize)] -struct WorkspaceRelaySendResponse { +struct ProjectRelaySendResponse { code: String, passphrase: String, - workspace: String, + project: String, + environment_count: usize, item_count: usize, } -/// Share an entire workspace (definition + decrypted values) via the relay. -async fn handle_workspace_relay_send( +/// Share a whole project (selected environments, structure + decrypted +/// values, deduped items) via the relay. `environment_ids` is the sender's +/// explicit selection — the GUI/CLI default non-default environments to +/// unchecked (D4), but that's a client-side safety default, not enforced +/// here; this endpoint shares exactly what it's asked to. +async fn handle_project_relay_send( State(state): State>, headers: HeaderMap, Path(id): Path, + Json(body): Json, ) -> impl IntoResponse { if let Err(code) = verify_token(&headers, &state).await { let (msg, err_code) = match code { @@ -2761,6 +3075,15 @@ async fn handle_workspace_relay_send( return err_json(code, msg, err_code).into_response(); } + if body.environment_ids.is_empty() { + return err_json( + StatusCode::UNPROCESSABLE_ENTITY, + "environment_ids must not be empty", + "VALIDATION_ERROR", + ) + .into_response(); + } + let (supabase_url, anon_key, bundle) = { let vault = state.vault.lock().await; let k = match vault.key.as_ref() { @@ -2802,98 +3125,27 @@ async fn handle_workspace_relay_send( } }; - let workspaces = match vault.db.list_workspaces().await { - Ok(w) => w, - Err(e) => { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") - .into_response() + let bundle = match project::relay::build_project_bundle(&vault.db, &k, id, &body.environment_ids).await { + Ok(b) => b, + Err(e) if e.contains("not found") => { + return err_json(StatusCode::NOT_FOUND, &e, "NOT_FOUND").into_response() } - }; - let ws = match workspaces.into_iter().find(|w| w.id == id) { - Some(w) => w, - None => { - return err_json(StatusCode::NOT_FOUND, "workspace not found", "NOT_FOUND") - .into_response() + Err(e) if e.contains("too large") => { + return err_json(StatusCode::PAYLOAD_TOO_LARGE, &e, "PAYLOAD_TOO_LARGE").into_response() } - }; - let ws_vars = match vault.db.get_workspace_vars(id).await { - Ok(v) => v, Err(e) => { return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") .into_response() } }; - // Decrypt every vault item once, keyed by id. - let raw = match vault.db.list_items().await { - Ok(r) => r, - Err(e) => { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") - .into_response() - } - }; - let mut items_by_id: std::collections::HashMap = - std::collections::HashMap::new(); - for (item_id, _, data, _, _) in &raw { - if let Ok(json) = crypto::decrypt(&k, data) { - if let Ok(item) = serde_json::from_slice::(&json) { - items_by_id.insert(*item_id, item); - } - } - } - - // Build manifest vars + the set of bundled items (deduped by name). - let mut bundle_vars: Vec = Vec::with_capacity(ws_vars.len()); - let mut bundled: std::collections::HashMap = - std::collections::HashMap::new(); - for v in &ws_vars { - match v.item_id { - Some(iid) => { - let item = match items_by_id.get(&iid) { - Some(it) => it, - // Referenced item was deleted — skip cleanly rather than fail. - None => continue, - }; - let item_name = item.name.clone().unwrap_or_default(); - bundled.entry(item_name.clone()).or_insert_with(|| PlainItem { - item_type: item.item_type.clone(), - name: item_name.clone(), - value: item.value.clone(), - username: item.username.clone(), - password: item.password.clone(), - url: item.url.clone(), - notes: item.notes.clone(), - category: item.categories.clone().and_then(|c| c.into_iter().next()), - command: item.command.clone(), - }); - bundle_vars.push(relay::WorkspaceBundleVar { - key: v.key.clone(), - item_name: Some(item_name), - literal: None, - }); - } - None => bundle_vars.push(relay::WorkspaceBundleVar { - key: v.key.clone(), - item_name: None, - literal: v.literal.clone(), - }), - } - } - - let bundle = relay::WorkspaceBundle { - kind: relay::WorkspaceBundle::KIND.to_string(), - name: ws.name.clone(), - description: ws.description.clone(), - template: ws.template.clone(), - vars: bundle_vars, - items: bundled.into_values().collect(), - }; - (supabase_url, anon_key, bundle) }; - let workspace_name = bundle.name.clone(); + let project_name = bundle.name.clone(); + let environment_count = bundle.environments.len(); let item_count = bundle.items.len(); + let code = relay::generate_share_code(); let passphrase = crate::share::crypto::generate_passphrase(); @@ -2904,7 +3156,7 @@ async fn handle_workspace_relay_send( .into_response() } }; - let payload = match relay::encrypt_workspace(&bundle, &relay_key) { + let payload = match relay::encrypt_project(&bundle, &relay_key) { Ok(p) => p, Err(e) => { return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string(), "INTERNAL_ERROR") @@ -2939,28 +3191,40 @@ async fn handle_workspace_relay_send( ( StatusCode::OK, - Json(WorkspaceRelaySendResponse { + Json(ProjectRelaySendResponse { code, passphrase, - workspace: workspace_name, + project: project_name, + environment_count, item_count, }), ) .into_response() } +#[derive(serde::Deserialize)] +struct ProjectRelayReceiveBody { + code: String, + passphrase: String, + #[serde(default)] + project_name_override: Option, +} + #[derive(serde::Serialize)] -struct WorkspaceRelayReceiveResponse { - workspace: String, - names: Vec, +struct ProjectRelayReceiveResponse { + project: String, + environments: Vec, + item_count: usize, } -/// Receive a complete workspace shared via the relay: recreate the bundled items, -/// then recreate the workspace and re-link its variables to the new items by name. -async fn handle_workspace_relay_receive( +/// Receive a whole project shared via the relay: recreates it as a brand-new +/// project (never merges into an existing one — D5), in one transaction +/// (D7). A case-insensitive project-name collision is reported as `409 +/// CONFLICT` so the caller can retry with `project_name_override`. +async fn handle_project_relay_receive( State(state): State>, headers: HeaderMap, - Json(body): Json, + Json(body): Json, ) -> impl IntoResponse { if let Err(code) = verify_token(&headers, &state).await { let (msg, err_code) = match code { @@ -3057,12 +3321,12 @@ async fn handle_workspace_relay_receive( } }; - let bundle = match relay::decrypt_workspace(&payload, &relay_key) { + let bundle = match relay::decrypt_project(&payload, &relay_key) { Ok(b) => b, Err(e) => { return err_json( StatusCode::UNPROCESSABLE_ENTITY, - &format!("decrypt failed (wrong passphrase or not a workspace?): {e}"), + &format!("decrypt failed (wrong passphrase or not a project package?): {e}"), "DECRYPT_ERROR", ) .into_response() @@ -3079,105 +3343,27 @@ async fn handle_workspace_relay_receive( .await; let vault = state.vault.lock().await; - let now_ts = now_ts_str(); - - // 1. Import bundled items, tracking name → new id so vars can re-link. - let mut id_by_name: std::collections::HashMap = std::collections::HashMap::new(); - let mut names: Vec = Vec::new(); - for plain in &bundle.items { - let vault_item = VaultItem { - id: 0, - item_type: plain.item_type.clone(), - name: Some(plain.name.clone()), - value: plain.value.clone(), - username: plain.username.clone(), - password: plain.password.clone(), - url: plain.url.clone(), - notes: plain.notes.clone(), - title: None, - description: None, - command: plain.command.clone(), - shell: None, - content: None, - categories: Some(plain.category.iter().cloned().collect()), - created: now_ts.clone(), - is_global: None, - }; - let json = match serde_json::to_vec(&vault_item) { - Ok(j) => j, - Err(e) => { - return err_json( - StatusCode::INTERNAL_SERVER_ERROR, - &format!("serialize item: {e}"), - "INTERNAL_ERROR", - ) - .into_response() - } - }; - let encrypted = match crypto::encrypt(&vault_key, &json) { - Ok(e) => e, - Err(e) => { - return err_json( - StatusCode::INTERNAL_SERVER_ERROR, - &format!("encrypt item: {e}"), - "INTERNAL_ERROR", - ) - .into_response() - } - }; - match vault - .db - .upsert_item(0, &vault_item.item_type, &encrypted, &vault_item.created, false) - .await - { - Ok(new_id) => { - id_by_name.insert(plain.name.clone(), new_id); - names.push(plain.name.clone()); - } - Err(e) => { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") - .into_response() - } - } - } + let result = project::relay::receive_project_bundle(&vault.db, &vault_key, bundle, body.project_name_override) + .await; - // 2. Recreate the workspace (no paths — receiver sets their own .env targets). - let ws_id = match vault - .db - .upsert_workspace(0, &bundle.name, bundle.description.as_deref(), &bundle.template) - .await - { - Ok(id) => id, - Err(e) => { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR") - .into_response() + match result { + Ok(r) => ( + StatusCode::OK, + Json(ProjectRelayReceiveResponse { + project: r.project_name, + environments: r.environment_names, + item_count: r.item_count, + }), + ) + .into_response(), + // Matches #12's "conflict:" string-prefix convention for db/project + // layer errors — see docs/plans/issue-4 D5. Swap for the shared + // `PROJECT_NAME_CONFLICT` constant once that branch merges. + Err(e) if e.starts_with("conflict:") => { + err_json(StatusCode::CONFLICT, &e, "CONFLICT").into_response() } - }; - - // 3. Re-link vars to the newly imported items by name. - let db_vars: Vec = bundle - .vars - .iter() - .map(|v| DbWorkspaceVar { - id: 0, - workspace_id: ws_id, - key: v.key.clone(), - item_id: v.item_name.as_ref().and_then(|n| id_by_name.get(n).copied()), - literal: v.literal.clone(), - }) - .collect(); - if let Err(e) = vault.db.set_workspace_vars(ws_id, &db_vars).await { - return err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR").into_response(); + Err(e) => err_json(StatusCode::INTERNAL_SERVER_ERROR, &e, "INTERNAL_ERROR").into_response(), } - - ( - StatusCode::OK, - Json(WorkspaceRelayReceiveResponse { - workspace: bundle.name, - names, - }), - ) - .into_response() } // ─── Función pública de arranque ────────────────────────────────────────────── @@ -3200,6 +3386,7 @@ pub(crate) fn build_router(state: Arc) -> Router { .route("/items/:id", put(handle_update_item)) .route("/items/:id", delete(handle_delete_item)) .route("/items/:id/reveal", post(handle_reveal_item)) + .route("/maintenance/orphans", get(handle_list_orphans)) .route("/categories", get(handle_list_categories)) .route("/categories", post(handle_create_category)) .route("/categories/:id", put(handle_update_category)) @@ -3225,8 +3412,8 @@ pub(crate) fn build_router(state: Arc) -> Router { .route("/environments/:id/example", post(handle_environment_example)) .route("/relay/send", post(handle_relay_send)) .route("/relay/receive", post(handle_relay_receive)) - .route("/workspaces/:id/relay/send", post(handle_workspace_relay_send)) - .route("/workspaces/relay/receive", post(handle_workspace_relay_receive)) + .route("/projects/:id/relay/send", post(handle_project_relay_send)) + .route("/projects/relay/receive", post(handle_project_relay_receive)) .with_state(state) .layer(middleware::from_fn(cors_guard)) } @@ -3267,3 +3454,160 @@ pub async fn start_server(vault: SharedState, app_data_dir: PathBuf) { #[cfg(test)] mod tests; + +// ─── Tests: issue #13, global-item scoped visibility ───────────────────────── +// +// Pure-function tests: plain `VaultItem` values and a `project::Environment` +// with synthetic `vars`, no database, no key, no async. `VaultItem` +// deliberately has no `#[derive(Debug)]` (it holds decrypted plaintext +// secrets — CLAUDE.md forbids secrets in logs/errors, and a stray `{:?}` on +// assertion failure would leak one into CI output), so assertions below +// compare individual fields rather than whole structs. +#[cfg(test)] +mod scope_tests { + use super::*; + + /// Builds a minimal `VaultItem` fixture. `is_global` mirrors the + /// `Option` shape of the real field (`None` behaves like `false` + /// for scoping purposes, same as `unwrap_or(false)` in `scope_items`). + fn item(id: i64, name: &str, is_global: Option) -> VaultItem { + VaultItem { + id, + item_type: "secret".to_string(), + name: Some(name.to_string()), + value: None, + url: None, + username: None, + password: None, + title: None, + description: None, + command: None, + shell: None, + categories: None, + notes: None, + content: None, + created: "2026-01-01T00:00:00Z".to_string(), + is_global, + } + } + + fn env_with_vars(pairs: &[(&str, i64)]) -> project::Environment { + project::Environment { + id: 1, + project_id: 1, + name: "test".to_string(), + is_default: true, + paths: vec![], + vars: pairs + .iter() + .map(|(key, item_id)| project::EnvironmentVar { + id: 0, + key: key.to_string(), + item_id: *item_id, + }) + .collect(), + created: "2026-01-01T00:00:00Z".to_string(), + updated: "2026-01-01T00:00:00Z".to_string(), + } + } + + #[test] + fn global_unlinked_item_visible_by_default() { + let item_a = item(1, "A", Some(false)); // linked, non-global + let item_b = item(2, "B", Some(true)); // unlinked, global + let env = env_with_vars(&[("A_KEY", 1)]); + let linked = environment_item_ids(&env); + + let result = scope_items(vec![item_a, item_b], &linked, IncludeGlobal::With); + + assert_eq!(result.len(), 2, "default mode must return both linked and unlinked-global items"); + let b = result.iter().find(|s| s.item.id == 2).expect("item B must be present"); + assert_eq!(b.item.is_global, Some(true)); + assert!(!b.linked, "unlinked global item must report linked: false"); + } + + #[test] + fn linked_item_reports_linked_true() { + let item_a = item(1, "A", Some(false)); + let env = env_with_vars(&[("A_KEY", 1)]); + let linked = environment_item_ids(&env); + + let result = scope_items(vec![item_a], &linked, IncludeGlobal::With); + + assert_eq!(result.len(), 1); + assert!(result[0].linked, "linked item must report linked: true"); + assert_eq!(result[0].item.is_global, Some(false)); + } + + #[test] + fn global_and_linked_item_appears_once() { + let item_c = item(3, "C", Some(true)); // both global and linked + let env = env_with_vars(&[("C_KEY", 3)]); + let linked = environment_item_ids(&env); + + let result = scope_items(vec![item_c], &linked, IncludeGlobal::With); + + assert_eq!(result.len(), 1, "item that is both global and linked must appear exactly once (dedup guard)"); + assert!(result[0].linked); + assert_eq!(result[0].item.is_global, Some(true)); + } + + #[test] + fn include_global_false_matches_legacy_scope() { + let item_a = item(1, "A", Some(false)); // linked, non-global + let item_b = item(2, "B", Some(true)); // unlinked, global + let env = env_with_vars(&[("A_KEY", 1)]); + let linked = environment_item_ids(&env); + + let result = scope_items(vec![item_a, item_b], &linked, IncludeGlobal::Without); + + assert_eq!(result.len(), 1, "Without must return exactly the linked set, matching the pre-change filter"); + assert_eq!(result[0].item.id, 1); + assert!(result[0].linked); + } + + #[test] + fn include_global_only_returns_globals_regardless_of_link() { + let item_a = item(1, "A", Some(false)); // linked, non-global + let item_b = item(2, "B", Some(true)); // unlinked, global + let item_c = item(3, "C", Some(true)); // linked, global + let env = env_with_vars(&[("A_KEY", 1), ("C_KEY", 3)]); + let linked = environment_item_ids(&env); + + let result = scope_items(vec![item_a, item_b, item_c], &linked, IncludeGlobal::Only); + + let ids: HashSet = result.iter().map(|s| s.item.id).collect(); + assert_eq!(ids, HashSet::from([2, 3]), "Only must return all is_global items regardless of linkage, excluding non-global A"); + } + + #[test] + fn search_and_type_filters_apply_to_unioned_globals() { + let item_a = item(1, "DB_HOST", Some(false)); // linked + let item_b = item(2, "API_KEY", Some(true)); // unlinked, global + let env = env_with_vars(&[("DB_HOST", 1)]); + let linked = environment_item_ids(&env); + + let scoped = scope_items(vec![item_a, item_b], &linked, IncludeGlobal::With); + assert_eq!(scoped.len(), 2, "union must include both before filtering"); + + let filtered = filter_scoped_items(scoped, None, None, Some("api")); + + assert_eq!(filtered.len(), 1, "search must narrow within the union, including unioned globals"); + assert_eq!(filtered[0].item.id, 2); + } + + #[test] + fn invalid_include_global_value_is_rejected() { + // `axum::response::Response` (the `Err` side) implements neither + // `Debug` nor `PartialEq`, so `assert_eq!`/`unwrap()` on the whole + // `Result` won't compile — `matches!` pattern-matches without + // requiring either trait. + assert!(IncludeGlobal::parse(Some("bogus")).is_err()); + assert!(matches!(IncludeGlobal::parse(None), Ok(IncludeGlobal::With))); + assert!(matches!(IncludeGlobal::parse(Some("true")), Ok(IncludeGlobal::With))); + assert!(matches!(IncludeGlobal::parse(Some("with")), Ok(IncludeGlobal::With))); + assert!(matches!(IncludeGlobal::parse(Some("false")), Ok(IncludeGlobal::Without))); + assert!(matches!(IncludeGlobal::parse(Some("without")), Ok(IncludeGlobal::Without))); + assert!(matches!(IncludeGlobal::parse(Some("only")), Ok(IncludeGlobal::Only))); + } +} diff --git a/src-tauri/src/api/tests/items.rs b/src-tauri/src/api/tests/items.rs index 9c749de..b639a3a 100644 --- a/src-tauri/src/api/tests/items.rs +++ b/src-tauri/src/api/tests/items.rs @@ -6,7 +6,11 @@ use crate::test_support::{read_item, req, router, unlocked_vault}; async fn list_scoped_to_environment_only_returns_linked_items() { let v = unlocked_vault().await; let app = router(&v); - let (status, json) = req(&app, "GET", &format!("/items?environment_id={}", v.env_id), Some(&v.token), None).await; + // `include_global=false` is the exact-linked set — the pre-issue-#13 + // filter, and what /fill and /inject will actually materialize. The + // default (`true`) now unions unlinked globals in; that union is covered + // by `api::scope_tests` and the #13 cases below. + let (status, json) = req(&app, "GET", &format!("/items?environment_id={}&include_global=false", v.env_id), Some(&v.token), None).await; assert_eq!(status.as_u16(), 200); let items = json.as_array().unwrap(); // 3 linked into "production"; the 4th seeded item (SHARED_TOKEN, global) diff --git a/src-tauri/src/api/tests/scope.rs b/src-tauri/src/api/tests/scope.rs index ca2bb65..15b3f9e 100644 --- a/src-tauri/src/api/tests/scope.rs +++ b/src-tauri/src/api/tests/scope.rs @@ -8,7 +8,7 @@ use crate::test_support::{link_var, locked_vault, req, router, seed_item, seed_p async fn resolves_by_environment_id() { let v = unlocked_vault().await; let app = router(&v); - let (status, json) = req(&app, "GET", &format!("/items?environment_id={}", v.env_id), Some(&v.token), None).await; + let (status, json) = req(&app, "GET", &format!("/items?environment_id={}&include_global=false", v.env_id), Some(&v.token), None).await; assert_eq!(status.as_u16(), 200u16); assert_eq!(json.as_array().unwrap().len(), 3); } @@ -17,7 +17,7 @@ async fn resolves_by_environment_id() { async fn resolves_by_project_and_environment_names() { let v = unlocked_vault().await; let app = router(&v); - let (status, json) = req(&app, "GET", "/items?project=demo&environment=production", Some(&v.token), None).await; + let (status, json) = req(&app, "GET", "/items?project=demo&environment=production&include_global=false", Some(&v.token), None).await; assert_eq!(status.as_u16(), 200u16); assert_eq!(json.as_array().unwrap().len(), 3); } diff --git a/src-tauri/src/bin/crypt-env-mcp.rs b/src-tauri/src/bin/crypt-env-mcp.rs index 348d6fd..8c1122e 100644 --- a/src-tauri/src/bin/crypt-env-mcp.rs +++ b/src-tauri/src/bin/crypt-env-mcp.rs @@ -202,6 +202,7 @@ fn tool_definitions() -> serde_json::Value { "properties": { "type": { "type": "string", "description": "Filter by type: secret, credential, link, command, note" }, "category": { "type": "string", "description": "Filter by category name" }, + "include_global": { "type": "string", "enum": ["true", "false", "only"], "description": "true (default) also lists reusable global secrets not yet linked into this environment — these appear with `linked: false` and will NOT be written by generate/inject/fill until linked. false restricts to items actually linked into this environment (what fill/inject will materialize). only returns just the global secrets, ignoring linkage." }, "environment_id": { "type": "integer", "description": "Environment ID (scope). Provide this, or both 'project' and 'environment'." }, "project": { "type": "string", "description": "Project name (case-insensitive). Used with 'environment' when 'environment_id' is not given." }, "environment": { "type": "string", "description": "Environment name within the project (case-insensitive), e.g. production, local, test. Used with 'project'." } @@ -226,6 +227,7 @@ fn tool_definitions() -> serde_json::Value { "type": "object", "properties": { "query": { "type": "string", "description": "Search term to match against item names" }, + "include_global": { "type": "string", "enum": ["true", "false", "only"], "description": "true (default) also searches reusable global secrets not yet linked into this environment — these appear with `linked: false` and will NOT be written by generate/inject/fill until linked. false restricts to items actually linked into this environment (what fill/inject will materialize). only returns just the global secrets, ignoring linkage." }, "environment_id": { "type": "integer", "description": "Environment ID (scope). Provide this, or both 'project' and 'environment'." }, "project": { "type": "string", "description": "Project name (case-insensitive). Used with 'environment' when 'environment_id' is not given." }, "environment": { "type": "string", "description": "Environment name within the project (case-insensitive), e.g. production, local, test. Used with 'project'." } @@ -263,7 +265,7 @@ fn tool_definitions() -> serde_json::Value { }, { "name": "crypt_env_add_item", - "description": "Adds a new item to the vault, owned by the given project and linked into the given environment. Requires scope: 'environment_id', or both 'project' and 'environment'.", + "description": "Adds a new item to the vault, owned by the given project and linked into the given environment. Requires scope: 'environment_id', or both 'project' and 'environment'. If the key already exists in the target environment, the existing item is updated in place (its previous value is destroyed) — this is the default ('on_conflict': 'update'). If that item is shared with other environments or projects (or is global), the call fails with a conflict instead of silently changing it elsewhere; retry with 'on_conflict': 'replace' to create a new item and repoint just this environment's link, or update the shared item explicitly with crypt_env_update_item. Set 'on_conflict': 'error' to fail on any existing key instead of updating it.", "inputSchema": { "type": "object", "properties": { @@ -275,6 +277,7 @@ fn tool_definitions() -> serde_json::Value { "url": { "type": "string" }, "username": { "type": "string" }, "key": { "type": "string", "description": "Environment variable key this item is linked under. Defaults to 'name' if omitted." }, + "on_conflict": { "type": "string", "enum": ["update", "replace", "error"], "description": "How to handle an existing item already linked under this key. 'update' (default): re-encrypt onto the existing item, destroying its previous value; fails if the item is shared elsewhere. 'replace': always create a new item and repoint this environment's link to it; the superseded item is deleted only if nothing else still references it. 'error': fail on any existing key." }, "environment_id": { "type": "integer", "description": "Environment ID (scope). Provide this, or both 'project' and 'environment'." }, "project": { "type": "string", "description": "Project name (case-insensitive). Used with 'environment' when 'environment_id' is not given." }, "environment": { "type": "string", "description": "Environment name within the project (case-insensitive), e.g. production, local, test. Used with 'project'." } @@ -576,29 +579,6 @@ fn tool_definitions() -> serde_json::Value { "required": ["code", "passphrase"] } }, - { - "name": "crypt_env_share_workspace_send", - "description": "Share a COMPLETE workspace via internet relay: its definition PLUS the decrypted values of every referenced secret, bundled into one encrypted package. The receiver reconstructs a ready-to-inject workspace in a single step — no need to share individual items. Returns a code and passphrase. IMPORTANT: Show the code and passphrase to the user immediately; the passphrase is only shown once. Identify the workspace by name or id.", - "inputSchema": { - "type": "object", - "properties": { - "id": { "type": "integer", "description": "Workspace ID" }, - "name": { "type": "string", "description": "Workspace name (case-insensitive, used if id not given)" } - } - } - }, - { - "name": "crypt_env_share_workspace_receive", - "description": "Receive a complete workspace shared via internet relay using a code and passphrase from the sender. Recreates the bundled secrets in the vault and rebuilds the workspace with its variables re-linked, ready to inject into a .env with crypt_env_inject_environment.", - "inputSchema": { - "type": "object", - "properties": { - "code": { "type": "string", "description": "Relay code provided by the sender (e.g. X7K2-M9P4)" }, - "passphrase": { "type": "string", "description": "Passphrase provided by the sender" } - }, - "required": ["code", "passphrase"] - } - }, { "name": "crypt_env_list_mcp_servers", "description": "List registered MCP servers from Claude config files. Returns name, command, args, and env key names — never env values.", @@ -689,7 +669,7 @@ fn tool_definitions() -> serde_json::Value { "properties": { "path": { "type": "string", "description": "Absolute path to the .env file to import, e.g. C:\\projects\\myapp\\.env" }, "category": { "type": "string", "description": "Optional category name to assign to all imported items" }, - "overwrite": { "type": "boolean", "description": "If true, update existing vault items that have the same name. Default: false (skip duplicates)." }, + "overwrite": { "type": "boolean", "description": "If true, update existing vault items that have the same name or environment key in place, destroying their previous value. Default: false (skip duplicates instead of erroring)." }, "environment_id": { "type": "integer", "description": "Environment ID (scope). Provide this, or both 'project' and 'environment'." }, "project": { "type": "string", "description": "Project name (case-insensitive). Used with 'environment' when 'environment_id' is not given." }, "environment": { "type": "string", "description": "Environment name within the project (case-insensitive), e.g. production, local, test. Used with 'project'." } @@ -868,6 +848,10 @@ fn tool_list_items(args: &serde_json::Value, token: &str) -> serde_json::Value { url.push_str(&format!("{}category={}", sep, cat)); sep = '&'; } + if let Some(ig) = args.get("include_global").and_then(|v| v.as_str()) { + url.push_str(&format!("{}include_global={}", sep, urlencod(ig))); + sep = '&'; + } append_scope_params(&mut url, &mut sep, args); let resp = match vault_get(&url, token) { @@ -902,6 +886,10 @@ fn tool_search_items(args: &serde_json::Value, token: &str) -> serde_json::Value let mut url = format!("/items?search={}", urlencod(&query)); let mut sep = '&'; + if let Some(ig) = args.get("include_global").and_then(|v| v.as_str()) { + url.push_str(&format!("{}include_global={}", sep, urlencod(ig))); + sep = '&'; + } append_scope_params(&mut url, &mut sep, args); let resp = match vault_get(&url, token) { @@ -1220,6 +1208,9 @@ fn tool_add_item(args: &serde_json::Value, token: &str) -> serde_json::Value { let mut url = "/items".to_string(); let mut sep = '?'; append_scope_params(&mut url, &mut sep, args); + if let Some(mode) = args.get("on_conflict").and_then(|v| v.as_str()) { + url.push_str(&format!("{sep}on_conflict={mode}")); + } let resp = match vault_post(&url, token, &body) { Ok(r) => r, @@ -1238,6 +1229,11 @@ fn tool_add_item(args: &serde_json::Value, token: &str) -> serde_json::Value { if status == 422 { return tool_err(format!("validation error (scope or field): {text}")); } + if status == 409 { + // SHARED_ITEM_CONFLICT / KEY_EXISTS / CONFLICT_RETRY — the response + // body already names the item and the remedy, never a secret value. + return tool_err(format!("conflict creating item: {text}")); + } if status >= 400 { return tool_err(format!("error creating item (HTTP {status}): {text}")); } @@ -2110,99 +2106,6 @@ fn tool_relay_receive(args: &serde_json::Value, token: &str) -> serde_json::Valu } } -fn tool_share_workspace_send(args: &serde_json::Value, token: &str) -> serde_json::Value { - // Resolve workspace ID: use id directly, or find by name (mirrors inject_workspace). - let workspace_id: i64 = if let Some(id) = args.get("id").and_then(|v| v.as_i64()) { - id - } else if let Some(name) = args.get("name").and_then(|v| v.as_str()) { - let resp = match vault_get("/workspaces", token) { - Ok(r) => r, - Err(e) => return tool_err(e), - }; - if resp.status().as_u16() == 403 { - return tool_err("vault_locked: unlock the vault first"); - } - let text = match resp.text() { - Ok(t) => t, - Err(e) => return tool_err(format!("error reading workspaces: {e}")), - }; - let list: serde_json::Value = match serde_json::from_str(&text) { - Ok(v) => v, - Err(_) => return tool_err("error parsing workspace list"), - }; - let name_lower = name.to_lowercase(); - let found = list.as_array().and_then(|arr| { - arr.iter().find(|ws| { - ws.get("name") - .and_then(|n| n.as_str()) - .map(|n| n.to_lowercase() == name_lower) - .unwrap_or(false) - }) - }).and_then(|ws| ws.get("id").and_then(|v| v.as_i64())); - match found { - Some(id) => id, - None => return tool_err(format!("workspace '{name}' not found")), - } - } else { - return tool_err("required: 'id' (integer) or 'name' (string)"); - }; - - let resp = match vault_post( - &format!("/workspaces/{workspace_id}/relay/send"), - token, - &serde_json::json!({}), - ) { - Ok(r) => r, - Err(e) => return tool_err(e), - }; - - let status = resp.status().as_u16(); - let text = match resp.text() { - Ok(t) => t, - Err(e) => return tool_err(format!("error reading response: {e}")), - }; - - if status == 403 { return tool_err("vault_locked: unlock the vault first"); } - if status == 404 { return tool_err(format!("workspace {workspace_id} not found")); } - if status >= 400 { return tool_err(format!("workspace share failed (HTTP {status}): {text}")); } - - match serde_json::from_str::(&text) { - Ok(v) => tool_ok(serde_json::to_string_pretty(&v).unwrap_or(text)), - Err(_) => tool_ok(text), - } -} - -fn tool_share_workspace_receive(args: &serde_json::Value, token: &str) -> serde_json::Value { - let code = match args.get("code").and_then(|v| v.as_str()) { - Some(c) => c.to_string(), - None => return tool_err("required parameter: 'code'"), - }; - let passphrase = match args.get("passphrase").and_then(|v| v.as_str()) { - Some(p) => p.to_string(), - None => return tool_err("required parameter: 'passphrase'"), - }; - - let body = serde_json::json!({ "code": code, "passphrase": passphrase }); - let resp = match vault_post("/workspaces/relay/receive", token, &body) { - Ok(r) => r, - Err(e) => return tool_err(e), - }; - - let status = resp.status().as_u16(); - let text = match resp.text() { - Ok(t) => t, - Err(e) => return tool_err(format!("error reading response: {e}")), - }; - - if status == 403 { return tool_err("vault_locked: unlock the vault first"); } - if status >= 400 { return tool_err(format!("workspace receive failed (HTTP {status}): {text}")); } - - match serde_json::from_str::(&text) { - Ok(v) => tool_ok(serde_json::to_string_pretty(&v).unwrap_or(text)), - Err(_) => tool_ok(text), - } -} - // ─── Dispatch ───────────────────────────────────────────────────────────────── // ─── Category tool implementations ─────────────────────────────────────────── @@ -3019,11 +2922,21 @@ fn tool_import_env_file(args: &serde_json::Value, token: &str) -> serde_json::Va updated += 1; keys.push(key.clone()); } else { - // Create new item, linked into the scoped environment. + // Create new item, linked into the scoped environment. The + // name-based search above can miss a key that is linked under a + // renamed item (name != key), so this POST can still collide on + // the environment key even when `existing` was None — map + // `overwrite` onto `on_conflict` explicitly rather than relying + // on the server default, so that path is covered too: + // overwrite=true -> update in place; overwrite=false -> error, + // treated below as a skip like the by-name check above. This is + // what keeps bulk import from mass-producing orphans (issue #9). let body = build_item_body(0, key, value, &category, &now_ts); let mut create_url = "/items".to_string(); let mut create_sep = '?'; append_scope_params(&mut create_url, &mut create_sep, args); + let on_conflict = if overwrite { "update" } else { "error" }; + create_url.push_str(&format!("{create_sep}on_conflict={on_conflict}")); let resp = match vault_post(&create_url, token, &body) { Ok(r) => r, Err(e) => { @@ -3039,6 +2952,12 @@ fn tool_import_env_file(args: &serde_json::Value, token: &str) -> serde_json::Va let text = resp.text().unwrap_or_default(); return tool_err(format!("scope required: pass 'environment_id', or both 'project' and 'environment' ({text})")); } + if status == 409 { + // Racing/renamed-item collision the name search couldn't see. + // Same report bucket as the by-name skip path above. + skipped_existing.push(key.clone()); + continue; + } if status >= 400 { let text = resp.text().unwrap_or_default(); errors.push(format!("{key}: create failed (HTTP {status}): {text}")); @@ -3124,8 +3043,6 @@ fn handle_tool_call(name: &str, args: &serde_json::Value, token: &str) -> serde_ "crypt_env_generate_example_env" => tool_generate_example_env(args, token), "crypt_env_relay_send" => tool_relay_send(args, token), "crypt_env_relay_receive" => tool_relay_receive(args, token), - "crypt_env_share_workspace_send" => tool_share_workspace_send(args, token), - "crypt_env_share_workspace_receive" => tool_share_workspace_receive(args, token), "crypt_env_list_mcp_servers" => tool_list_mcp_servers(args, token), "crypt_env_add_mcp_server" => tool_add_mcp_server(args, token), "crypt_env_update_mcp_server" => tool_update_mcp_server(args, token), diff --git a/src-tauri/src/bin/crypt-env/client.rs b/src-tauri/src/bin/crypt-env/client.rs index f7e5d6e..be845b9 100644 --- a/src-tauri/src/bin/crypt-env/client.rs +++ b/src-tauri/src/bin/crypt-env/client.rs @@ -61,9 +61,17 @@ pub struct ItemSummary { pub title: Option, #[serde(default)] pub categories: Vec, + /// Present on `/items` responses since issue #13 (defaults to `false` + /// when absent, e.g. responses from before this change). + #[serde(default, rename = "isGlobal")] + pub is_global: bool, + /// `true` iff this item is linked into the queried environment. Also + /// new since issue #13 — defaults to `false` when absent. + #[serde(default)] + pub linked: bool, } -#[derive(Deserialize, Debug)] +#[derive(Deserialize, Debug, Clone)] #[allow(dead_code)] pub struct CommandDetail { pub id: i64, @@ -76,6 +84,43 @@ pub struct CommandDetail { pub command: Option, #[serde(default)] pub placeholders: Vec, + /// Present on `/commands` list responses since issue #13. `GET + /// /commands/:id` (unscoped, untouched by this change) omits it, so this + /// defaults to `false` there. + #[serde(default, rename = "isGlobal")] + pub is_global: bool, + /// `true` iff this command is linked into the queried environment. + /// Meaningless outside a scoped `/commands` list — defaults to `false`. + #[serde(default)] + pub linked: bool, +} + +/// Given the full `/commands` list for a scope, finds the command matching +/// `name` case-insensitively. When both a linked command and a global +/// (unlinked) command share the same name, the linked one wins — matching +/// `/fill`'s and `/inject`'s materialization-only semantics — and a warning +/// naming the shadowed global's id is printed to stderr. +pub fn resolve_command_by_name(commands: Vec, name: &str) -> Option { + let name_lower = name.to_lowercase(); + let mut matches: Vec = commands + .into_iter() + .filter(|c| c.name.to_lowercase() == name_lower) + .collect(); + + if matches.len() > 1 { + if let Some(linked_idx) = matches.iter().position(|c| c.linked) { + let linked = matches.remove(linked_idx); + for shadowed in matches.iter().filter(|c| !c.linked) { + eprintln!( + "warning: command '{}' also exists as a global item (id {}) — using the linked one", + name, shadowed.id + ); + } + return Some(linked); + } + } + + matches.into_iter().next() } #[derive(Deserialize, Debug)] diff --git a/src-tauri/src/bin/crypt-env/commands/add.rs b/src-tauri/src/bin/crypt-env/commands/add.rs index d2ecf98..257863d 100644 --- a/src-tauri/src/bin/crypt-env/commands/add.rs +++ b/src-tauri/src/bin/crypt-env/commands/add.rs @@ -118,19 +118,32 @@ pub fn run(args: AddArgs) -> Result<(), CliError> { .map(|(k, _)| k.clone()) .collect(); + // Server default (`on_conflict=update`, omitted below) now does what this + // prompt has always claimed: re-encrypts onto the existing item in place + // instead of orphaning it. Declining no longer silently drops the key + // client-side — it is still sent, but with `on_conflict=error`, so a + // collision that is still there by request time is rejected loudly + // rather than the CLI pretending nothing was asked for it. This also + // covers the race where the key stopped existing between the `existing` + // check above and the request (the server always re-checks). + let mut declined_keys: HashSet = HashSet::new(); if !conflict_keys.is_empty() && !args.force { // Show only key names — never values eprintln!("The following keys already exist: {}", conflict_keys.join(", ")); if !crate::prompts::confirm("Update all conflicting keys?") { - let conflict_set: HashSet<&str> = - conflict_keys.iter().map(|s| s.as_str()).collect(); - pairs.retain(|(k, _)| !conflict_set.contains(k.as_str())); + declined_keys = conflict_keys.into_iter().collect(); } } - let items_url = resolved_scope.append_query(&format!("{}/items", client::API_BASE)); + let base_items_url = resolved_scope.append_query(&format!("{}/items", client::API_BASE)); for (key, value) in &pairs { + let items_url = if declined_keys.contains(key) { + format!("{base_items_url}&on_conflict=error") + } else { + base_items_url.clone() + }; + let body = serde_json::json!({ "id": 0, "type": item_type, @@ -141,9 +154,24 @@ pub fn run(args: AddArgs) -> Result<(), CliError> { "key": key, }); let resp = client::authenticated_post(&items_url, &body)?; - if !resp.status().is_success() { + let status = resp.status(); + if status == reqwest::StatusCode::CONFLICT { + // Surface the conflict reason, not a bare HTTP 409 — only the + // key name and server-provided reason, never the value. + let (msg, code) = resp + .json::() + .ok() + .and_then(|v| { + Some(( + v.get("error")?.as_str()?.to_string(), + v.get("code")?.as_str()?.to_string(), + )) + }) + .unwrap_or_else(|| ("conflict".to_string(), "CONFLICT".to_string())); + eprintln!("Conflict for '{key}' [{code}]: {msg}"); + } else if !status.is_success() { // Only key name in error — never the value - eprintln!("Failed to add '{}': HTTP {}", key, resp.status()); + eprintln!("Failed to add '{}': HTTP {}", key, status); } else { eprintln!("Added: {} ({} / {})", key, resolved_scope.project, resolved_scope.environment); } diff --git a/src-tauri/src/bin/crypt-env/commands/cmd.rs b/src-tauri/src/bin/crypt-env/commands/cmd.rs index 04c5d0e..ff9ca22 100644 --- a/src-tauri/src/bin/crypt-env/commands/cmd.rs +++ b/src-tauri/src/bin/crypt-env/commands/cmd.rs @@ -84,11 +84,8 @@ fn command_info(name: &str, resolved_scope: &ResolvedScope) -> Result<(), CliErr } let commands: Vec = resp.json().map_err(|e| CliError::Api(e.to_string()))?; - let name_lower = name.to_lowercase(); - let found = commands - .into_iter() - .find(|c| c.name.to_lowercase() == name_lower); + let found = client::resolve_command_by_name(commands, name); let cmd_id = match found { Some(c) => c.id, @@ -133,11 +130,8 @@ fn run_command(name: &str, vars: &[String], resolved_scope: &ResolvedScope) -> R } let commands: Vec = resp.json().map_err(|e| CliError::Api(e.to_string()))?; - let name_lower = name.to_lowercase(); - let cmd = commands - .into_iter() - .find(|c| c.name.to_lowercase() == name_lower) + let cmd = client::resolve_command_by_name(commands, name) .ok_or_else(|| CliError::NotFound(name.to_string()))?; let mut template = cmd.command.unwrap_or_default(); diff --git a/src-tauri/src/bin/crypt-env/commands/exec.rs b/src-tauri/src/bin/crypt-env/commands/exec.rs index 0e58e90..dc3897f 100644 --- a/src-tauri/src/bin/crypt-env/commands/exec.rs +++ b/src-tauri/src/bin/crypt-env/commands/exec.rs @@ -39,11 +39,8 @@ pub fn run(args: ExecArgs) -> Result<(), CliError> { } let commands: Vec = resp.json().map_err(|e| CliError::Api(e.to_string()))?; - let name_lower = args.name.to_lowercase(); - let cmd = commands - .into_iter() - .find(|c| c.name.to_lowercase() == name_lower) + let cmd = client::resolve_command_by_name(commands, &args.name) .ok_or_else(|| CliError::NotFound(args.name.clone()))?; let mut template = cmd.command.unwrap_or_default(); diff --git a/src-tauri/src/bin/crypt-env/commands/list.rs b/src-tauri/src/bin/crypt-env/commands/list.rs index 94f03ea..e3a6bc2 100644 --- a/src-tauri/src/bin/crypt-env/commands/list.rs +++ b/src-tauri/src/bin/crypt-env/commands/list.rs @@ -21,16 +21,31 @@ pub struct ListArgs { /// Environment name (defaults to crypt-env.json or the project's default environment) #[arg(long = "env")] pub env: Option, + + /// Whether to include reusable global commands not linked into this + /// environment: `with` (default) unions them in, `without` matches + /// pre-issue-13 behaviour (linked commands only), `only` returns globals + /// regardless of linkage. + #[arg(long = "scope-globals", default_value = "with")] + pub scope_globals: String, } pub fn run(args: ListArgs) -> Result<(), CliError> { let resolved_scope = scope::resolve(args.project.as_deref(), args.env.as_deref(), false)?; - let url = resolved_scope.append_query(&format!("{}/commands", client::API_BASE)); + let url = resolved_scope.append_query(&format!( + "{}/commands?include_global={}", + client::API_BASE, + client::urlencod(&args.scope_globals) + )); let resp = client::authenticated_get(&url)?; if resp.status() == reqwest::StatusCode::FORBIDDEN { return Err(CliError::VaultLocked); } + if resp.status() == reqwest::StatusCode::UNPROCESSABLE_ENTITY { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("invalid --scope-globals value: {text}"))); + } if !resp.status().is_success() { return Err(CliError::Api(format!("HTTP {}", resp.status()))); } @@ -61,7 +76,7 @@ pub fn run(args: ListArgs) -> Result<(), CliError> { table .load_preset(UTF8_FULL) .set_content_arrangement(ContentArrangement::Dynamic) - .set_header(vec!["Name", "Description", "Shell", "Placeholders"]); + .set_header(vec!["Name", "Description", "Shell", "Placeholders", "Scope"]); for cmd in &commands { let placeholders = cmd.placeholders.join(", "); @@ -70,9 +85,20 @@ pub fn run(args: ListArgs) -> Result<(), CliError> { cmd.description.as_deref().unwrap_or(""), cmd.shell.as_deref().unwrap_or(""), &placeholders, + scope_label(cmd.is_global, cmd.linked), ]); } println!("{table}"); Ok(()) } + +/// `linked`/`global`/`global+linked` discriminator for the Scope column. +fn scope_label(is_global: bool, linked: bool) -> &'static str { + match (linked, is_global) { + (true, true) => "global+linked", + (true, false) => "linked", + (false, true) => "global", + (false, false) => "", + } +} diff --git a/src-tauri/src/bin/crypt-env/commands/project.rs b/src-tauri/src/bin/crypt-env/commands/project.rs index ab783cb..c9e55ab 100644 --- a/src-tauri/src/bin/crypt-env/commands/project.rs +++ b/src-tauri/src/bin/crypt-env/commands/project.rs @@ -1,8 +1,10 @@ +use std::collections::HashMap; + use clap::{Args, Subcommand}; use comfy_table::{presets::UTF8_FULL, ContentArrangement, Table}; use serde::Deserialize; -use crate::client::{authenticated_delete, authenticated_get, authenticated_post, CliError, API_BASE}; +use crate::client::{authenticated_delete, authenticated_get, authenticated_post, CliError, ItemSummary, API_BASE}; // ─── CLI argument structs ───────────────────────────────────────────────────── @@ -40,21 +42,51 @@ pub enum ProjectCmd { #[arg(long)] id: i64, }, + /// Share a whole project (structure + values, selected environments) via internet relay + Share { + /// Project ID (numeric) + #[arg(long, conflicts_with = "name")] + id: Option, + /// Project name (case-insensitive, used if --id is not given) + #[arg(long)] + name: Option, + /// Comma-separated environment names to include (default: the project's + /// default environment only). Pass "all" to include every environment. + #[arg(long)] + envs: Option, + /// Skip the confirmation prompt + #[arg(long)] + yes: bool, + }, + /// Receive a project shared via internet relay — always creates a new project + Receive { + /// Relay code (e.g. X7K2-M9P4) + #[arg(long)] + code: String, + /// Passphrase provided by the sender + #[arg(long)] + passphrase: String, + /// Project name to use instead of the sender's, if it collides with an existing one + #[arg(long = "as")] + as_name: Option, + }, } // ─── Response types ─────────────────────────────────────────────────────────── #[derive(Deserialize, Debug)] -#[allow(dead_code)] struct EnvironmentVar { key: String, + #[serde(rename = "itemId")] + item_id: i64, } #[derive(Deserialize, Debug)] -#[allow(dead_code)] struct EnvironmentSummary { id: i64, name: String, + #[serde(default, rename = "isDefault")] + is_default: bool, #[serde(default)] paths: Vec, #[serde(default)] @@ -85,6 +117,8 @@ pub fn run(args: ProjectArgs) -> Result<(), CliError> { ProjectCmd::Inject { id, project, environment } => run_inject(id, project, environment), ProjectCmd::Delete { id } => run_delete(id), ProjectCmd::DeleteEnv { id } => run_delete_env(id), + ProjectCmd::Share { id, name, envs, yes } => run_share(id, name, envs, yes), + ProjectCmd::Receive { code, passphrase, as_name } => run_receive(code, passphrase, as_name), } } @@ -231,3 +265,214 @@ fn run_delete_env(id: i64) -> Result<(), CliError> { println!("Environment {id} deleted."); Ok(()) } + +// ─── Share ──────────────────────────────────────────────────────────────────── +// Any environment whose name looks production-like gets a distinct warning in +// the manifest before the confirmation prompt (D4) — a heuristic, not a hard +// gate: it only ever adds friction, never removes it. +const PROD_NAME_PATTERN: &[&str] = &["prod", "production", "live", "release"]; + +fn looks_production(env_name: &str) -> bool { + let lower = env_name.to_lowercase(); + PROD_NAME_PATTERN.iter().any(|p| lower.contains(p)) +} + +fn resolve_project(id: Option, name: Option<&str>) -> Result { + let projects = fetch_projects()?; + if let Some(id) = id { + return projects.into_iter().find(|p| p.id == id).ok_or_else(|| CliError::NotFound(format!("project {id}"))); + } + if let Some(name) = name { + let name_lower = name.to_lowercase(); + return projects + .into_iter() + .find(|p| p.name.to_lowercase() == name_lower) + .ok_or_else(|| CliError::NotFound(format!("project '{name}'"))); + } + Err(CliError::Api("provide --id or --name to identify the project".into())) +} + +/// Selects which environments to include, given `--envs` (comma-separated +/// names, or "all"). Omitted defaults to the project's default environment +/// only — the same safety default as the GUI's checklist (D4): under-sharing +/// costs one extra round trip, over-sharing is unrecoverable. +fn select_environments<'a>(project: &'a ProjectSummary, envs: Option<&str>) -> Result, CliError> { + if project.environments.is_empty() { + return Err(CliError::Api(format!("project '{}' has no environments", project.name))); + } + + match envs { + None => { + let default_env = project + .environments + .iter() + .find(|e| e.is_default) + .unwrap_or(&project.environments[0]); + Ok(vec![default_env]) + } + Some("all") => Ok(project.environments.iter().collect()), + Some(list) => { + let mut selected = Vec::new(); + for want in list.split(',').map(|s| s.trim()).filter(|s| !s.is_empty()) { + let want_lower = want.to_lowercase(); + let env = project + .environments + .iter() + .find(|e| e.name.to_lowercase() == want_lower) + .ok_or_else(|| CliError::NotFound(format!("environment '{want}' in project '{}'", project.name)))?; + selected.push(env); + } + if selected.is_empty() { + return Err(CliError::Api("--envs resolved to no environments".into())); + } + Ok(selected) + } + } +} + +/// Fetches item metadata (name, never value) linked into `environment_id`, so +/// the manifest can show `KEY -> item name` instead of raw item ids — reuses +/// the existing `GET /items` endpoint rather than adding a new one (D4). +fn fetch_environment_item_names(environment_id: i64) -> Result, CliError> { + let resp = authenticated_get(&format!("{API_BASE}/items?environment_id={environment_id}"))?; + if resp.status() == reqwest::StatusCode::FORBIDDEN { + return Err(CliError::VaultLocked); + } + if !resp.status().is_success() { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("list items failed: {text}"))); + } + let items: Vec = resp.json().map_err(|e| CliError::Api(e.to_string()))?; + Ok(items + .into_iter() + .map(|i| (i.id, i.name.or(i.title).unwrap_or_else(|| format!("#{}", i.id)))) + .collect()) +} + +fn run_share(id: Option, name: Option, envs: Option, yes: bool) -> Result<(), CliError> { + let project = resolve_project(id, name.as_deref())?; + let selected = select_environments(&project, envs.as_deref())?; + + println!( + "About to share project '{}' ({} environment{}):", + project.name, + selected.len(), + if selected.len() != 1 { "s" } else { "" } + ); + for env in &selected { + let warn = if looks_production(&env.name) { " /!\\ PRODUCTION-LIKE NAME" } else { "" }; + println!(" {}{}", env.name, warn); + if env.vars.is_empty() { + println!(" (no variables)"); + continue; + } + let names = fetch_environment_item_names(env.id)?; + for v in &env.vars { + let item_name = names.get(&v.item_id).cloned().unwrap_or_else(|| format!("#{}", v.item_id)); + println!(" {} -> {}", v.key, item_name); + } + } + println!(); + println!("Values leave this machine via the encrypted relay. Never shown here."); + + if !yes { + print!("Proceed? [y/N]: "); + if !prompt_confirm()? { + println!("Cancelled."); + return Ok(()); + } + } + + let environment_ids: Vec = selected.iter().map(|e| e.id).collect(); + let body = serde_json::json!({ "environment_ids": environment_ids }); + let resp = authenticated_post(&format!("{API_BASE}/projects/{}/relay/send", project.id), &body)?; + + if resp.status() == reqwest::StatusCode::FORBIDDEN { + return Err(CliError::VaultLocked); + } + if resp.status() == reqwest::StatusCode::NOT_FOUND { + return Err(CliError::NotFound(format!("project {}", project.id))); + } + if !resp.status().is_success() { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("project share failed: {text}"))); + } + + #[derive(Deserialize)] + struct ShareResult { + code: String, + passphrase: String, + environment_count: usize, + item_count: usize, + } + let result: ShareResult = resp.json().map_err(|e| CliError::Api(e.to_string()))?; + + println!(); + println!("╔══════════════════════════════════════════════════════════╗"); + println!("║ RELAY SHARE CODE AND PASSPHRASE — show to recipient now ║"); + println!("╠══════════════════════════════════════════════════════════╣"); + println!("║ Code: {:<46} ║", result.code); + println!("║ Passphrase: {:<46} ║", result.passphrase); + println!("╚══════════════════════════════════════════════════════════╝"); + println!(); + println!( + "{} environment(s), {} item(s). The passphrase is shown once. Provide both to the recipient.", + result.environment_count, result.item_count + ); + + Ok(()) +} + +// ─── Receive ────────────────────────────────────────────────────────────────── + +fn run_receive(code: String, passphrase: String, as_name: Option) -> Result<(), CliError> { + let mut body = serde_json::json!({ "code": code, "passphrase": passphrase }); + if let Some(name) = &as_name { + body["project_name_override"] = serde_json::json!(name); + } + + let resp = authenticated_post(&format!("{API_BASE}/projects/relay/receive"), &body)?; + + if resp.status() == reqwest::StatusCode::FORBIDDEN { + return Err(CliError::VaultLocked); + } + if resp.status() == reqwest::StatusCode::CONFLICT { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("{text} — retry with --as NAME to choose a different project name"))); + } + if !resp.status().is_success() { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("project receive failed: {text}"))); + } + + #[derive(Deserialize)] + struct ReceiveResult { + project: String, + environments: Vec, + item_count: usize, + } + let result: ReceiveResult = resp.json().map_err(|e| CliError::Api(e.to_string()))?; + + println!( + "Received project '{}' — {} environment(s), {} item(s):", + result.project, + result.environments.len(), + result.item_count + ); + for name in &result.environments { + println!(" + {name}"); + } + println!("Run `crypt-env project list` to see it — set paths on each environment before injecting."); + + Ok(()) +} + +// ─── Prompt helper ──────────────────────────────────────────────────────────── + +fn prompt_confirm() -> Result { + use std::io::{self, Write}; + io::stdout().flush().ok(); + let mut input = String::new(); + io::stdin().read_line(&mut input).map_err(CliError::Io)?; + Ok(matches!(input.trim().to_lowercase().as_str(), "y" | "yes")) +} diff --git a/src-tauri/src/bin/crypt-env/commands/search.rs b/src-tauri/src/bin/crypt-env/commands/search.rs index 9438c10..b0fe35c 100644 --- a/src-tauri/src/bin/crypt-env/commands/search.rs +++ b/src-tauri/src/bin/crypt-env/commands/search.rs @@ -14,20 +14,32 @@ pub struct SearchArgs { /// Environment name (defaults to crypt-env.json or the project's default environment) #[arg(long = "env")] pub env: Option, + + /// Whether to include reusable global items not linked into this + /// environment: `with` (default) unions them in, `without` matches + /// pre-issue-13 behaviour (linked items only), `only` returns globals + /// regardless of linkage. + #[arg(long = "scope-globals", default_value = "with")] + pub scope_globals: String, } pub fn run(args: SearchArgs) -> Result<(), CliError> { let resolved_scope = scope::resolve(args.project.as_deref(), args.env.as_deref(), false)?; let url = resolved_scope.append_query(&format!( - "{}/items?search={}", + "{}/items?search={}&include_global={}", client::API_BASE, - client::urlencod(&args.query) + client::urlencod(&args.query), + client::urlencod(&args.scope_globals) )); let resp = client::authenticated_get(&url)?; if resp.status() == reqwest::StatusCode::FORBIDDEN { return Err(CliError::VaultLocked); } + if resp.status() == reqwest::StatusCode::UNPROCESSABLE_ENTITY { + let text = resp.text().unwrap_or_default(); + return Err(CliError::Api(format!("invalid --scope-globals value: {text}"))); + } if !resp.status().is_success() { let code = resp.status(); return Err(CliError::Api(format!("HTTP error {code}"))); @@ -40,8 +52,8 @@ pub fn run(args: SearchArgs) -> Result<(), CliError> { return Ok(()); } - println!("{:<6} {:<16} {:<32} CATEGORIES", "ID", "TYPE", "NAME/TITLE"); - println!("{}", "-".repeat(80)); + println!("{:<6} {:<16} {:<32} {:<16} CATEGORIES", "ID", "TYPE", "NAME/TITLE", "SCOPE"); + println!("{}", "-".repeat(96)); for item in &items { let display_name = item .name @@ -49,13 +61,24 @@ pub fn run(args: SearchArgs) -> Result<(), CliError> { .or(item.title.as_deref()) .unwrap_or(""); println!( - "{:<6} {:<16} {:<32} {}", + "{:<6} {:<16} {:<32} {:<16} {}", item.id, item.item_type, display_name, + scope_label(item.is_global, item.linked), item.categories.join(", ") ); } Ok(()) } + +/// `linked`/`global`/`global+linked` discriminator for the SCOPE column. +fn scope_label(is_global: bool, linked: bool) -> &'static str { + match (linked, is_global) { + (true, true) => "global+linked", + (true, false) => "linked", + (false, true) => "global", + (false, false) => "", + } +} diff --git a/src-tauri/src/db/mod.rs b/src-tauri/src/db/mod.rs index dfe0d12..de6ba0f 100644 --- a/src-tauri/src/db/mod.rs +++ b/src-tauri/src/db/mod.rs @@ -87,6 +87,94 @@ struct RenameRecord { at: String, } +// ─── Issue #9: create-or-update on env-key collision ────────────────────── +// +// `db` knows only ids and counts here — no HTTP status codes (that mapping +// lives in `api`) and no ciphertext (crypto lives in `vault`). See +// `create_or_link_item` for the transactional write this classification +// feeds. + +/// Read-only snapshot of the item currently linked to a `(environment_id, +/// key)` pair, returned by `inspect_env_key`. Never carries `items.data`. +#[derive(Debug, Clone)] +pub struct EnvKeyConflict { + pub item_id: i64, + pub created: String, + pub is_global: bool, + /// Rows in `environment_vars` pointing at `item_id` (including this one). + pub link_count: i64, + pub owner_ids: Vec, +} + +/// How `create_or_link_item` should behave on a collision. Mirrors the +/// `on_conflict` query parameter accepted by `POST /items`. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum LinkMode { + /// Re-encrypt onto the existing row when it is exclusively owned by the + /// calling project and linked nowhere else; otherwise conflict. + Update, + /// Always create a new row and repoint the link; delete the superseded + /// item only if it is now unreachable (unlinked and non-global). + Replace, + /// Any collision at all is rejected — strict create semantics. + Error, +} + +/// Outcome of `create_or_link_item`. +#[derive(Debug, Clone)] +pub enum LinkOutcome { + Created { item_id: i64, is_global: bool }, + Updated { item_id: i64, is_global: bool }, + Conflict { item_id: i64, reason: ConflictReason }, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConflictReason { + /// The colliding item is linked elsewhere, multi-owned, or global — + /// mutating it in place would change a value seen outside the caller's + /// project+environment. + Shared, + /// `on_conflict=error` (or an `Error`-mode caller) rejects any collision. + KeyExists, + /// The transactional re-check found a different `item_id` linked than + /// the one the caller's earlier `inspect_env_key` call saw. + StateChanged, +} + +// ─── Project-relay receive (issue #4) ───────────────────────────────────────── +// Plain-data input to `insert_received_project` — the `db` layer never sees a +// vault key or a `PlainItem`, only ciphertext strings the caller (the +// `project`/`share` layers) already produced. See CLAUDE.md's module rule: +// `db` does not know about encryption. + +/// One item to insert, already encrypted by the caller. `name` is used only +/// to resolve `ReceivedVar::item_name` references within this same call — +/// it is never stored (the vault item's name lives inside its ciphertext). +pub struct ReceivedProjectItem { + pub name: String, + pub item_type: String, + pub ciphertext: String, + pub created: String, +} + +pub struct ReceivedVar { + pub key: String, + pub item_name: String, +} + +pub struct ReceivedEnvironment { + pub name: String, + pub is_default: bool, + pub vars: Vec, +} + +#[derive(Debug, Serialize, Clone)] +pub struct InsertedProject { + pub project_id: i64, + pub environment_ids: Vec, + pub item_ids: Vec, +} + pub struct VaultDb { pool: SqlitePool, path: String, @@ -153,6 +241,14 @@ impl VaultDb { let stmts = [ "PRAGMA journal_mode=WAL", "PRAGMA foreign_keys=ON", + // SQLite zeroes freed page content on every DELETE/UPDATE-that-frees + // when this is on, instead of merely unlinking it. Covers `delete_item`, + // `delete_project`'s cascades, the `Replace` conflict branch (§3.2), and + // the orphan prune path uniformly, rather than relying on call sites + // remembering to zero-then-delete (see `wipe_and_reset`'s file-level + // version of the same idea). Connection-level setting, no on-disk + // format impact — safe to remove at any time. + "PRAGMA secure_delete=ON", "CREATE TABLE IF NOT EXISTS vault_meta ( id INTEGER PRIMARY KEY CHECK(id = 1), kdf_salt TEXT NOT NULL, @@ -1532,6 +1628,432 @@ impl VaultDb { .map_err(|e| e.to_string()) } + // ─── Issue #9: create-or-update on env-key collision ────────────────── + + /// Read-only snapshot of the item currently linked to `key` in + /// `environment_id`, plus enough shape (link/owner counts) for the caller + /// to classify it as exclusive vs shared without decrypting anything. + /// Never selects `items.data` — no ciphertext leaves this call. + pub async fn inspect_env_key( + &self, + environment_id: i64, + key: &str, + ) -> Result, String> { + let row = sqlx::query( + "SELECT i.id, i.created, i.is_global + FROM environment_vars ev + JOIN items i ON i.id = ev.item_id + WHERE ev.environment_id = ?1 AND ev.key = ?2", + ) + .bind(environment_id) + .bind(key) + .fetch_optional(&self.pool) + .await + .map_err(|e| e.to_string())?; + + // No row at all, or a legacy `literal`-only var (`item_id IS NULL`, so + // the JOIN drops it) → nothing to update in place. The create path + // runs and `upsert_environment_var`'s repoint replaces the literal, + // matching `migrate_literal_vars_to_items`'s upgrade semantics. + let row = match row { + Some(r) => r, + None => return Ok(None), + }; + + let item_id: i64 = row.get(0); + let created: String = row.get(1); + let is_global_i: i64 = row.get(2); + + let link_count: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM environment_vars WHERE item_id = ?1") + .bind(item_id) + .fetch_one(&self.pool) + .await + .map_err(|e| e.to_string())?; + + let owner_ids = self.list_owning_projects(item_id).await?; + + Ok(Some(EnvKeyConflict { + item_id, + created, + is_global: is_global_i != 0, + link_count, + owner_ids, + })) + } + + /// The single transactional mutation behind `POST /items`'s on-conflict + /// handling. Receives an already-encrypted blob (crypto lives in `vault`, + /// never here) and performs the whole read-classify-write inside one + /// `sqlx::Transaction`, so there is no interleaving that leaves an item + /// owned but unlinked (M3) or a `Replace` delete racing its own repoint. + /// + /// `expected` is the `item_id` (if any) the caller saw during its earlier + /// (non-transactional) `inspect_env_key` call. The inspection is re-run + /// here, inside the transaction; if the current state disagrees, the + /// write is rejected as `Conflict { StateChanged }` rather than acting on + /// a classification that may no longer hold. In practice all writers + /// serialize on the process-wide `SharedState` mutex, so this is defence + /// in depth, not the primary mechanism. + #[allow(clippy::too_many_arguments)] + pub async fn create_or_link_item( + &self, + environment_id: i64, + project_id: i64, + key: &str, + item_type: &str, + encrypted: &str, + created: &str, + mode: LinkMode, + expected: Option, + ) -> Result { + let mut tx = self.pool.begin().await.map_err(|e| e.to_string())?; + + let current = sqlx::query( + "SELECT i.id, i.is_global + FROM environment_vars ev + JOIN items i ON i.id = ev.item_id + WHERE ev.environment_id = ?1 AND ev.key = ?2", + ) + .bind(environment_id) + .bind(key) + .fetch_optional(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + // Carry `is_global` alongside the id so the occupied-key branch below + // reads it straight off the matched row instead of re-unwrapping + // `current` (bare `unwrap()` is forbidden in production code). + let current_row = current + .as_ref() + .map(|r| (r.get::(0), r.get::(1) != 0)); + let current_item_id = current_row.map(|(id, _)| id); + + if current_item_id != expected { + tx.rollback().await.map_err(|e| e.to_string())?; + // `item_id` in the response is best-effort: whichever id the + // caller already knew about, for the error message. + return Ok(LinkOutcome::Conflict { + item_id: expected.or(current_item_id).unwrap_or(0), + reason: ConflictReason::StateChanged, + }); + } + + let now = now_ts(); + + match current_row { + None => { + // Free key → create, own, link. + let res = sqlx::query( + "INSERT INTO items (item_type, data, created, updated, is_global) VALUES (?1, ?2, ?3, ?4, 0)", + ) + .bind(item_type) + .bind(encrypted) + .bind(created) + .bind(&now) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + let new_id = res.last_insert_rowid(); + + sqlx::query("INSERT OR IGNORE INTO item_projects (item_id, project_id) VALUES (?1, ?2)") + .bind(new_id) + .bind(project_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + sqlx::query( + "INSERT INTO environment_vars (environment_id, key, item_id, literal) VALUES (?1, ?2, ?3, NULL) + ON CONFLICT(environment_id, key) DO UPDATE SET item_id = excluded.item_id, literal = NULL", + ) + .bind(environment_id) + .bind(key) + .bind(new_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + tx.commit().await.map_err(|e| e.to_string())?; + Ok(LinkOutcome::Created { item_id: new_id, is_global: false }) + } + Some((item_id, is_global)) => { + + match mode { + LinkMode::Error => { + tx.rollback().await.map_err(|e| e.to_string())?; + Ok(LinkOutcome::Conflict { item_id, reason: ConflictReason::KeyExists }) + } + LinkMode::Update => { + let link_count: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM environment_vars WHERE item_id = ?1") + .bind(item_id) + .fetch_one(&mut *tx) + .await + .map_err(|e| e.to_string())?; + let owner_rows = sqlx::query("SELECT project_id FROM item_projects WHERE item_id = ?1") + .bind(item_id) + .fetch_all(&mut *tx) + .await + .map_err(|e| e.to_string())?; + let owner_ids: Vec = owner_rows.into_iter().map(|r| r.get(0)).collect(); + + let exclusive = !is_global && link_count == 1 && owner_ids == [project_id]; + + if !exclusive { + tx.rollback().await.map_err(|e| e.to_string())?; + return Ok(LinkOutcome::Conflict { item_id, reason: ConflictReason::Shared }); + } + + sqlx::query("UPDATE items SET data = ?1, updated = ?2 WHERE id = ?3") + .bind(encrypted) + .bind(&now) + .bind(item_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + tx.commit().await.map_err(|e| e.to_string())?; + Ok(LinkOutcome::Updated { item_id, is_global: false }) + } + LinkMode::Replace => { + let res = sqlx::query( + "INSERT INTO items (item_type, data, created, updated, is_global) VALUES (?1, ?2, ?3, ?4, 0)", + ) + .bind(item_type) + .bind(encrypted) + .bind(created) + .bind(&now) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + let new_id = res.last_insert_rowid(); + + sqlx::query("INSERT OR IGNORE INTO item_projects (item_id, project_id) VALUES (?1, ?2)") + .bind(new_id) + .bind(project_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + sqlx::query("UPDATE environment_vars SET item_id = ?1, literal = NULL WHERE environment_id = ?2 AND key = ?3") + .bind(new_id) + .bind(environment_id) + .bind(key) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + // Delete only if now unreachable — a property of the + // statement (NOT EXISTS), not of application logic + // that could drift. Safe no-op when `item_id` is still + // linked elsewhere or is global. + sqlx::query( + "DELETE FROM items WHERE id = ?1 AND is_global = 0 + AND NOT EXISTS (SELECT 1 FROM environment_vars WHERE item_id = ?1)", + ) + .bind(item_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + sqlx::query( + "DELETE FROM item_projects WHERE item_id = ?1 + AND NOT EXISTS (SELECT 1 FROM items WHERE id = ?1)", + ) + .bind(item_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + tx.commit().await.map_err(|e| e.to_string())?; + Ok(LinkOutcome::Created { item_id: new_id, is_global: false }) + } + } + } + } + } + + /// The issue's reproduction query, verbatim: ids of items with zero + /// `environment_vars` references and `is_global = 0`. Global items have a + /// reachable surface (Global Secrets) even when unlinked; non-global + /// unlinked items have none — that asymmetry is why the predicate omits + /// `is_global = 1` rows. See plan §4.6 for the condition under which this + /// predicate would need to gain a `item_projects` clause (it does not + /// today — re-verified against `main` immediately before this shipped). + pub async fn list_orphan_item_ids(&self) -> Result, String> { + let rows = sqlx::query( + "SELECT i.id FROM items i + LEFT JOIN environment_vars ev ON ev.item_id = i.id + WHERE ev.id IS NULL AND i.is_global = 0", + ) + .fetch_all(&self.pool) + .await + .map_err(|e| e.to_string())?; + Ok(rows.into_iter().map(|r| r.get(0)).collect()) + } + + /// Deletes `item_projects` then `items` for the given ids, in one + /// transaction, re-checking the orphan predicate per id inside the + /// transaction so a concurrently re-linked item is skipped rather than + /// destroyed underneath its new link. + pub async fn delete_items_cascade(&self, ids: &[i64]) -> Result<(), String> { + let mut tx = self.pool.begin().await.map_err(|e| e.to_string())?; + for id in ids { + // Re-check the full orphan predicate (unlinked AND non-global) per + // id, inside the transaction: a concurrently re-linked item, or + // one promoted to global, is skipped rather than destroyed + // underneath its new reachability. + sqlx::query( + "DELETE FROM item_projects WHERE item_id = ?1 + AND EXISTS (SELECT 1 FROM items WHERE id = ?1 AND is_global = 0) + AND NOT EXISTS (SELECT 1 FROM environment_vars WHERE item_id = ?1)", + ) + .bind(id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + sqlx::query( + "DELETE FROM items WHERE id = ?1 AND is_global = 0 + AND NOT EXISTS (SELECT 1 FROM environment_vars WHERE item_id = ?1)", + ) + .bind(id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + } + tx.commit().await.map_err(|e| e.to_string()) + } + + /// Inserts an entire received project (from a relay `ProjectBundle`) in + /// one transaction: project row, items (already-encrypted ciphertext, + /// never decrypted here), `item_projects` ownership, environments and + /// `environment_vars` — all-or-nothing (issue #4 D7). A failure partway + /// through must not leave orphaned rows, since the relay payload has + /// already been burned by the time this runs and cannot be re-fetched. + /// + /// Checks for a case-insensitive project-name collision **before** + /// writing anything, returning a `"conflict: ..."`-prefixed error the + /// caller maps to HTTP 409 — this is an application-level, Unicode-aware + /// pre-check (`str::to_lowercase`), not a reliance on the DB's ASCII-only + /// `NOCASE` unique index, so it (a) never surfaces a raw SQLite + /// constraint-violation string and (b) also catches non-ASCII collisions + /// the index would miss. TODO(#12): once `idx_projects_name_nocase`'s + /// shared collision helper / `PROJECT_NAME_CONFLICT` constant lands, + /// swap this loop for it instead of duplicating the convention. + pub async fn insert_received_project( + &self, + name: &str, + description: Option<&str>, + template: &str, + items: &[ReceivedProjectItem], + environments: &[ReceivedEnvironment], + ) -> Result { + let existing_names: Vec = sqlx::query_scalar("SELECT name FROM projects") + .fetch_all(&self.pool) + .await + .map_err(|e| e.to_string())?; + let name_lower = name.to_lowercase(); + if existing_names.iter().any(|n| n.to_lowercase() == name_lower) { + // Named error (D5): the caller (Tauri command / HTTP handler) has + // no other way to learn which name collided, since the bundle + // that carried it has already been decrypted server-side and + // will be burned from the relay before any retry — so the name + // is echoed back here rather than only asserting a generic + // conflict, letting the GUI pre-fill a rename suggestion. + return Err(format!("conflict: a project named '{name}' already exists")); + } + + let mut tx = self.pool.begin().await.map_err(|e| e.to_string())?; + let now = now_ts(); + + let project_id = sqlx::query( + "INSERT INTO projects (name, description, template, created, updated) VALUES (?1,?2,?3,?4,?5)", + ) + .bind(name) + .bind(description) + .bind(template) + .bind(&now) + .bind(&now) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())? + .last_insert_rowid(); + + // Items — ciphertext only, is_global = false (D7: provenance is one + // project; the receiver opts in to global explicitly afterwards). + let mut item_id_by_name: HashMap = HashMap::new(); + let mut item_ids = Vec::with_capacity(items.len()); + for it in items { + let item_id = sqlx::query( + "INSERT INTO items (item_type, data, created, updated, is_global) VALUES (?1,?2,?3,?4,0)", + ) + .bind(&it.item_type) + .bind(&it.ciphertext) + .bind(&it.created) + .bind(&now) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())? + .last_insert_rowid(); + + sqlx::query("INSERT INTO item_projects (item_id, project_id) VALUES (?1, ?2)") + .bind(item_id) + .bind(project_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + + item_id_by_name.insert(it.name.clone(), item_id); + item_ids.push(item_id); + } + + // Environments + vars, resolving each var's item_name against the + // items just inserted above. + let mut environment_ids = Vec::with_capacity(environments.len()); + for env in environments { + let is_default_i: i64 = if env.is_default { 1 } else { 0 }; + let env_id = sqlx::query( + "INSERT INTO environments (project_id, name, is_default, created, updated) VALUES (?1,?2,?3,?4,?5)", + ) + .bind(project_id) + .bind(&env.name) + .bind(is_default_i) + .bind(&now) + .bind(&now) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())? + .last_insert_rowid(); + + for v in &env.vars { + // Defensive only — the bundle builder never emits a var + // whose item_name isn't also in `items` (see + // `project::relay::build_project_bundle`), so this should + // never actually skip anything in practice. + let item_id = match item_id_by_name.get(&v.item_name) { + Some(id) => *id, + None => continue, + }; + sqlx::query( + "INSERT INTO environment_vars (environment_id, key, item_id, literal) VALUES (?1,?2,?3,NULL)", + ) + .bind(env_id) + .bind(&v.key) + .bind(item_id) + .execute(&mut *tx) + .await + .map_err(|e| e.to_string())?; + } + + environment_ids.push(env_id); + } + + tx.commit().await.map_err(|e| e.to_string())?; + + Ok(InsertedProject { project_id, environment_ids, item_ids }) } + pub async fn wipe_and_reset(&mut self) -> Result<(), String> { self.pool.close().await; // Sobreescribir contenido con ceros antes de eliminar (mitigación forense básica) diff --git a/src-tauri/src/fsguard/mod.rs b/src-tauri/src/fsguard/mod.rs new file mode 100644 index 0000000..68c287c --- /dev/null +++ b/src-tauri/src/fsguard/mod.rs @@ -0,0 +1,313 @@ +//! Filesystem containment guard. +//! +//! Dependency-free leaf module (imports only `std`) — see issue #7 and +//! `docs/plans/issue-7-path-traversal-environment-name.md` §4/D7 for why this +//! lives on its own rather than inside `api` or `project`: it is needed by +//! both and has no dependencies of its own, so a leaf module is the only +//! shape that does not create a sideways dependency between peers. +//! +//! This module answers exactly one question: *given a caller-trusted base +//! directory and an untrusted single filename, what is the one real path +//! that filename is allowed to resolve to?* It never decides whether a write +//! to that path should proceed (see #8's `guarded_write`, which composes on +//! top of this). + +use std::fs; +use std::path::{Component, Path, PathBuf}; + +/// Why `resolve_within` refused to produce a path. `Display` on this type +/// describes the rule that was broken, never the input that broke it — the +/// input is attacker-controlled stored data (see plan §4/D5). +#[derive(Debug, PartialEq, Eq)] +pub enum ContainmentError { + /// `file_name` was empty or whitespace-only. + EmptyName, + /// `file_name` did not lexically resolve to exactly one `Normal` path + /// component — separators, `.`, `..`, an absolute/UNC/verbatim prefix, + /// or a Windows drive-relative form (`C:foo`) all land here. + NotASingleComponent, + /// `file_name` contained a NUL byte or another control character. + NulByte, + /// `file_name` is (ignoring an extension) a Windows reserved device + /// name: `CON`, `PRN`, `AUX`, `NUL`, `COM1`-`COM9`, `LPT1`-`LPT9`. + ReservedDeviceName, + /// `file_name` ends with a trailing `.` or space, contains `:` (NTFS + /// alternate data streams), or contains one of the other Win32-illegal + /// characters `< > " | ? *`. + TrailingDotOrSpace, + /// The base directory itself could not be created or canonicalized. + /// Carries the raw io error text — this is about the caller-supplied + /// base, never about the untrusted name, so it is safe to surface. + BaseUnusable(String), + /// The joined (or, for an existing target, re-canonicalized) path did + /// not stay under the canonicalized base. This is the belt-and-braces + /// check: reaching it means every lexical rule above already passed and + /// something else — most plausibly a pre-planted symlink — is trying to + /// redirect the write. + Escapes, +} + +impl std::fmt::Display for ContainmentError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ContainmentError::EmptyName => { + write!(f, "file name must not be empty or whitespace-only") + } + ContainmentError::NotASingleComponent => write!( + f, + "file name must be a single path component: no separators, no '.', no '..', and no drive/UNC/verbatim prefix" + ), + ContainmentError::NulByte => { + write!(f, "file name must not contain a NUL byte or control characters") + } + ContainmentError::ReservedDeviceName => write!( + f, + "file name must not be a reserved device name (CON, PRN, AUX, NUL, COM1-9, LPT1-9)" + ), + ContainmentError::TrailingDotOrSpace => write!( + f, + "file name must not end with a trailing '.' or space, and must not contain ':', '<', '>', '\"', '|', '?', or '*'" + ), + ContainmentError::BaseUnusable(msg) => write!(f, "base directory is not usable: {msg}"), + ContainmentError::Escapes => write!(f, "resolved path escapes the base directory"), + } + } +} + +const RESERVED_DEVICE_NAMES: &[&str] = &[ + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", + "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", +]; + +fn is_reserved_device_name(file_name: &str) -> bool { + // Reserved names apply to the base name (before the first '.'), both + // bare and with any extension — `NUL`, `nul.txt` and `Nul.tar.gz` are + // all the same reserved device on Windows. + let base = file_name.split('.').next().unwrap_or(file_name); + RESERVED_DEVICE_NAMES.iter().any(|r| r.eq_ignore_ascii_case(base)) +} + +/// Resolves `file_name` as a direct child of `base_dir`, guaranteeing the +/// result cannot be anywhere else. `file_name` is treated as a literal +/// filename: it is never decoded, unescaped, or normalized. +/// +/// Steps, in order (see the plan for the full rationale of each): +/// 1. Reject empty/whitespace, NUL, control characters. +/// 2. Lexical single-component check (rejects `..`, `.`, absolute/UNC/ +/// verbatim/drive-relative forms) plus an explicit separator check that +/// also catches `\`-based traversal on platforms where `Path` does not +/// treat `\` as a separator. +/// 3. Windows-hostile-name check, applied on every platform: reserved +/// device names, trailing dot/space, NTFS alternate-data-stream `:`, and +/// the rest of the Win32-illegal character set. +/// 4. Create and canonicalize the base — the only `create_dir_all` call in +/// this function, and its argument is `base_dir` alone, never anything +/// with `file_name` interpolated into it. +/// 5. Join and assert the result is component-wise contained in the +/// canonicalized base. +/// 6. If the target already exists, re-canonicalize it and re-assert +/// containment — catches a pre-planted symlink. +pub fn resolve_within(base_dir: &str, file_name: &str) -> Result { + // 1. Empty / whitespace / NUL / control characters. + if file_name.trim().is_empty() { + return Err(ContainmentError::EmptyName); + } + if file_name.contains('\0') || file_name.chars().any(|c| c.is_control()) { + return Err(ContainmentError::NulByte); + } + + // 2. Lexical single-component check. + let components: Vec = Path::new(file_name).components().collect(); + let is_single_normal = components.len() == 1 && matches!(components[0], Component::Normal(_)); + if !is_single_normal { + return Err(ContainmentError::NotASingleComponent); + } + // Belt: on Unix, `Path` does not treat `\` as a separator, so + // `..\..\x` would otherwise parse as one `Normal` component. + if file_name.contains('/') || file_name.contains('\\') { + return Err(ContainmentError::NotASingleComponent); + } + + // 3. Windows-hostile-name check, applied on every platform. + if is_reserved_device_name(file_name) { + return Err(ContainmentError::ReservedDeviceName); + } + if file_name.ends_with('.') || file_name.ends_with(' ') { + return Err(ContainmentError::TrailingDotOrSpace); + } + if file_name.contains(':') || file_name.chars().any(|c| matches!(c, '<' | '>' | '"' | '|' | '?' | '*')) { + return Err(ContainmentError::TrailingDotOrSpace); + } + + // 4. Base resolution. This is the only `create_dir_all` in the whole + // flow, and it never sees `file_name`. + fs::create_dir_all(base_dir).map_err(|e| ContainmentError::BaseUnusable(e.to_string()))?; + let real_base = fs::canonicalize(base_dir).map_err(|e| ContainmentError::BaseUnusable(e.to_string()))?; + + // 5. Join and verify. `Path::starts_with` compares whole components, so + // it cannot be fooled by string-prefix tricks (`/base` vs `/base-evil`). + let target = real_base.join(file_name); + if !target.starts_with(&real_base) { + return Err(ContainmentError::Escapes); + } + + // 6. Symlink post-check: if something already sits at `target` (e.g. a + // pre-planted symlink from an earlier attack attempt), re-resolve it and + // re-assert containment. If it does not exist, step 4 already proved + // the parent directory is real and step 5's join is authoritative. + if target.exists() { + match fs::canonicalize(&target) { + Ok(resolved) if resolved.starts_with(&real_base) => {} + _ => return Err(ContainmentError::Escapes), + } + } + + Ok(target) +} + +#[cfg(test)] +mod tests { + use super::*; + + // Pure-lexical cases only — no filesystem I/O. See + // `tests/path_containment.rs` for the filesystem-level invariants + // (base creation, symlink escapes, the malicious-name table, etc). + + #[test] + fn rejects_empty_and_whitespace() { + assert_eq!( + resolve_within("/tmp", "").unwrap_err(), + ContainmentError::EmptyName + ); + assert_eq!( + resolve_within("/tmp", " ").unwrap_err(), + ContainmentError::EmptyName + ); + } + + #[test] + fn rejects_nul_and_control_chars() { + assert_eq!( + resolve_within("/tmp", "prod\0evil").unwrap_err(), + ContainmentError::NulByte + ); + assert_eq!( + resolve_within("/tmp", "prod\nevil").unwrap_err(), + ContainmentError::NulByte + ); + } + + #[test] + fn rejects_parent_and_current_dir() { + assert_eq!( + resolve_within("/tmp", "..").unwrap_err(), + ContainmentError::NotASingleComponent + ); + assert_eq!( + resolve_within("/tmp", ".").unwrap_err(), + ContainmentError::NotASingleComponent + ); + assert_eq!( + resolve_within("/tmp", "../../../tmp/pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + } + + #[test] + fn rejects_windows_traversal_even_on_unix() { + // `\` is not a separator to `Path` on Unix, so without the explicit + // belt check this would otherwise parse as one `Normal` component. + assert_eq!( + resolve_within("/tmp", "..\\..\\..\\Windows\\Temp\\pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + } + + #[test] + fn rejects_absolute_paths() { + assert_eq!( + resolve_within("/tmp", "/etc/cron.d/pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + } + + #[test] + fn rejects_windows_absolute_and_unc_and_verbatim() { + assert_eq!( + resolve_within("/tmp", "C:\\Windows\\Temp\\pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + assert_eq!( + resolve_within("/tmp", "\\\\wsl.localhost\\Ubuntu\\home\\u\\pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + assert_eq!( + resolve_within("/tmp", "\\\\?\\C:\\pwned").unwrap_err(), + ContainmentError::NotASingleComponent + ); + } + + #[test] + fn rejects_drive_relative() { + // On Unix this lexically parses as one `Normal` component (no + // separator, `Path` does not know about drive letters) — the + // Windows-hostile-name check's `:` rejection is what actually + // catches it there, which is why the assertion is just "is_err". + assert!(resolve_within("/tmp", "C:pwned").is_err()); + } + + #[test] + fn rejects_reserved_device_names() { + for name in ["CON", "NUL", "COM1", "LPT1", "con.txt", "Nul.tar.gz"] { + assert_eq!( + resolve_within("/tmp", name).unwrap_err(), + ContainmentError::ReservedDeviceName, + "expected {name} to be rejected as a reserved device name" + ); + } + } + + #[test] + fn rejects_trailing_dot_or_space() { + assert_eq!( + resolve_within("/tmp", "prod.").unwrap_err(), + ContainmentError::TrailingDotOrSpace + ); + assert_eq!( + resolve_within("/tmp", "prod ").unwrap_err(), + ContainmentError::TrailingDotOrSpace + ); + } + + #[test] + fn rejects_ads_and_illegal_chars() { + assert_eq!( + resolve_within("/tmp", "env:stream").unwrap_err(), + ContainmentError::TrailingDotOrSpace + ); + for name in ["ab", "a\"b", "a|b", "a?b", "a*b"] { + assert_eq!( + resolve_within("/tmp", name).unwrap_err(), + ContainmentError::TrailingDotOrSpace, + "expected {name} to be rejected" + ); + } + } + + #[test] + fn does_not_decode_percent_or_unicode_lookalikes() { + // These must be treated as literal filenames. They are not path + // separators or control characters, so — as documented in the + // plan's T1 — a single-component result contained under the base is + // an acceptable outcome for this layer; silently *decoding* them is + // the actually-forbidden behaviour, and that would show up as a + // `NotASingleComponent` rejection here, which none of these trigger. + for name in ["%2e%2e%2fpwned", "..%c0%af..%c0%afpwned", "../pwned"] { + let result = resolve_within("/tmp", name); + assert!( + result.is_err() || matches!(&result, Ok(p) if p.parent().is_some()), + "unexpected outcome for {name}: {result:?}" + ); + } + } +} diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 009c6e2..9bef615 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -7,6 +7,7 @@ pub mod cli; pub mod crypto; pub mod db; pub mod envfile; +pub mod fsguard; pub mod mcp; pub mod project; pub mod share; @@ -25,7 +26,7 @@ use vault::{ vault_get_settings, vault_import_backup, vault_import_backup_data, vault_import_items, vault_is_setup, vault_list, vault_lock, vault_parse_import, vault_save_categories, vault_save_item, vault_save_settings, vault_unlock, vault_wipe, vault_create_project_item, vault_set_item_global, - vault_get_item_owners, SharedState, VaultState, + vault_get_item_owners, vault_list_orphan_items, vault_prune_orphan_items, SharedState, VaultState, }; use vault::share_commands::{ share_cancel, share_confirm_fingerprint, share_export_file, share_import_file, @@ -37,6 +38,7 @@ use project::{ project_delete, project_export, project_import, project_list, project_pick_env_path, project_preview_delete, project_save, }; +use project::relay_commands::{project_relay_receive, project_relay_send}; use wsl::{wsl_distro_home, wsl_list_distros}; struct PendingUpdate(std::sync::Mutex>); @@ -184,6 +186,8 @@ pub fn run() { vault_create_project_item, vault_set_item_global, vault_get_item_owners, + vault_list_orphan_items, + vault_prune_orphan_items, vault_get_categories, vault_save_categories, vault_get_settings, @@ -224,6 +228,8 @@ pub fn run() { wsl_list_distros, wsl_distro_home, environment_inject_preview, + project_relay_send, + project_relay_receive, check_for_update, install_update, app_is_first_run, diff --git a/src-tauri/src/project/mod.rs b/src-tauri/src/project/mod.rs index c9bc3dd..7cf7645 100644 --- a/src-tauri/src/project/mod.rs +++ b/src-tauri/src/project/mod.rs @@ -7,6 +7,9 @@ use crate::db::{DbEnvironmentVar, ProjectDeleteImpact, VaultDb}; use crate::envfile; use crate::vault::SharedState; +pub mod relay; +pub mod relay_commands; + // ─── Frontend-facing types ──────────────────────────────────────────────────── /// Every variable is a real vault item now — no more bare literals. `item_id` @@ -177,9 +180,104 @@ async fn category_names_to_ids(db: &VaultDb, names: &[String]) -> Result Result<(), String> { + if name.trim().is_empty() { + return Err("must not be empty or whitespace-only".to_string()); + } + if name.contains('\0') || name.chars().any(|c| c.is_control()) { + return Err("must not contain control characters or a NUL byte".to_string()); + } + if name.contains('/') || name.contains('\\') { + return Err("must not contain '/' or '\\'".to_string()); + } + if name == "." || name == ".." { + return Err("must not be '.' or '..'".to_string()); + } + if name.contains(':') || name.chars().any(|c| matches!(c, '<' | '>' | '"' | '|' | '?' | '*')) { + return Err("must not contain ':', '<', '>', '\"', '|', '?', or '*'".to_string()); + } + if name.ends_with('.') || name.ends_with(' ') { + return Err("must not end with a trailing '.' or space".to_string()); + } + const RESERVED: &[&str] = &[ + "CON", "PRN", "AUX", "NUL", "COM1", "COM2", "COM3", "COM4", "COM5", "COM6", "COM7", "COM8", + "COM9", "LPT1", "LPT2", "LPT3", "LPT4", "LPT5", "LPT6", "LPT7", "LPT8", "LPT9", + ]; + let base = name.split('.').next().unwrap_or(name); + if RESERVED.iter().any(|r| r.eq_ignore_ascii_case(base)) { + return Err("must not be a reserved device name (CON, PRN, AUX, NUL, COM1-9, LPT1-9)".to_string()); + } + Ok(()) +} + +/// Strict allowlist: `^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$`, plus an explicit +/// rejection of a trailing `.` or `-`. Environment names are machine +/// identifiers, land in a filename (`.env.`), and are the field with +/// the proven traversal exploit (issue #7) — the tight rule costs nothing +/// real for names like `production`, `local`, `staging-2`. +pub fn validate_environment_name(name: &str) -> Result<(), String> { + const RULE: &str = + "must be 1-64 chars, start with a letter or digit, and contain only letters, digits, '.', '_' or '-'"; + if reject_filesystem_hostile(name).is_err() { + return Err(format!("name: {RULE}")); + } + let starts_alnum = name.chars().next().is_some_and(|c| c.is_ascii_alphanumeric()); + let charset_ok = name.chars().all(|c| c.is_ascii_alphanumeric() || matches!(c, '.' | '_' | '-')); + let valid = !name.is_empty() + && name.len() <= 64 + && starts_alnum + && charset_ok + && !name.ends_with('.') + && !name.ends_with('-'); + if valid { + Ok(()) + } else { + Err(format!("name: {RULE}")) + } +} + +/// Deny-list, deliberately laxer than the environment rule: rejects the +/// filesystem-hostile core above, plus a length cap and leading dot/ +/// whitespace. Allows spaces and non-ASCII letters — project names are human +/// labels ("Mi Proyecto" is a perfectly normal edit to an existing project) +/// and an ASCII-only allowlist would reject real edits for no present +/// security gain (see plan §4/D2). +pub fn validate_project_name(name: &str) -> Result<(), String> { + if let Err(reason) = reject_filesystem_hostile(name) { + return Err(format!("name: {reason}")); + } + if name.chars().count() > 128 { + return Err("name: must be 128 characters or fewer".to_string()); + } + if name.starts_with('.') || name.starts_with(' ') { + return Err("name: must not start with '.' or whitespace".to_string()); + } + Ok(()) +} + /// Creates (id = 0) or updates (id > 0) a project's metadata. A newly created /// project always gets one 'default' environment so it's immediately usable. pub async fn save_project(db: &VaultDb, input: ProjectInput) -> Result { + validate_project_name(&input.name)?; let is_new = input.id == 0; let project_id = db .upsert_project(input.id, &input.name, input.description.as_deref(), &input.template) @@ -239,8 +337,12 @@ async fn ensure_no_case_collision( /// when that item is global. A local item can never silently gain a second /// owner through this path; the caller must mark it global first. pub async fn save_environment(db: &VaultDb, input: EnvironmentInput) -> Result { + // Order matters: reject a structurally invalid name (issue #7) before + // spending a query on the collision check (issue #12). + validate_environment_name(&input.name)?; ensure_no_case_collision(db, input.project_id, input.id, &input.name).await?; + let env_id = db .upsert_environment(input.id, input.project_id, &input.name, input.is_default) .await?; @@ -403,8 +505,16 @@ async fn resolve_and_inspect( } } else if resolved.is_empty() { if let Some(dir) = output_dir { - let dir = dir.trim_end_matches(['/', '\\']); - resolved.push((format!("{dir}/.env.{}", env.name), PathOrigin::CallerSupplied)); + // Contained resolution (issue #7): the environment name is + // stored, untrusted data — it may only pick the filename inside + // `dir`, never redirect the write elsewhere. This is the same + // sink `/fill` and `/environments/:id/example` guard in + // `api::mod`; issue #8 moved the resolution in here, so the + // containment check has to live here too rather than at the + // former call site. + let target = crate::fsguard::resolve_within(dir.as_str(), &format!(".env.{}", env.name)) + .map_err(|e| format!("output_dir: {e}"))?; + resolved.push((target.to_string_lossy().into_owned(), PathOrigin::CallerSupplied)); } } diff --git a/src-tauri/src/project/relay.rs b/src-tauri/src/project/relay.rs new file mode 100644 index 0000000..065aae6 --- /dev/null +++ b/src-tauri/src/project/relay.rs @@ -0,0 +1,215 @@ +//! Whole-project sharing via the encrypted relay (issue #4). Two pure +//! orchestrators shared by the HTTP handlers (`api::mod`) and the Tauri +//! commands (`project::relay_commands`), following the same +//! decoupling CLAUDE.md requires elsewhere: this module encrypts/decrypts +//! with the vault key and hands `db` only plain data (ciphertext strings, +//! never the key) — see `db::insert_received_project`. + +use std::collections::HashMap; + +use crate::db::{ReceivedEnvironment, ReceivedProjectItem, ReceivedVar, VaultDb}; +use crate::share::package::PlainItem; +use crate::share::relay::{EnvironmentBundle, ProjectBundle, ProjectBundleVar}; +use crate::vault::VaultItem; + +use super::list_projects; + +/// Refuse to build a bundle whose pre-encryption JSON exceeds this size +/// (D8). The relay's actual request-size limit isn't verifiable from this +/// repo, so this is a deterministic local guard rather than a guess at a +/// third party's limit — a 200-item project lands around 100 KiB, so 1 MiB +/// is comfortable headroom for realistic projects. +const MAX_BUNDLE_BYTES: usize = 1024 * 1024; + +fn now_ts() -> String { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap_or_default() + .as_secs() + .to_string() +} + +/// Builds a `ProjectBundle` for `project_id`, restricted to `environment_ids`. +/// Items referenced by more than one selected environment are deduped by +/// name and hoisted to the bundle root (D1); an `environment_vars` row whose +/// `item_id` no longer resolves to a real item is skipped rather than +/// failing the whole send (mirrors the pre-existing item-relay behaviour). +pub async fn build_project_bundle( + db: &VaultDb, + vault_key: &[u8; 32], + project_id: i64, + environment_ids: &[i64], +) -> Result { + let project = list_projects(db) + .await? + .into_iter() + .find(|p| p.id == project_id) + .ok_or_else(|| "project not found".to_string())?; + + let selected: Vec<_> = project + .environments + .into_iter() + .filter(|e| environment_ids.contains(&e.id)) + .collect(); + + if selected.is_empty() { + return Err("no environments selected".to_string()); + } + + // Decrypt every vault item once, keyed by id (mirrors the existing + // item-relay send handler's pattern). + let raw = db.list_items().await?; + let mut items_by_id: HashMap = HashMap::new(); + for (item_id, _, data, _, _) in &raw { + if let Ok(json) = crate::crypto::decrypt(vault_key, data) { + if let Ok(item) = serde_json::from_slice::(&json) { + items_by_id.insert(*item_id, item); + } + } + } + + let mut bundled: HashMap = HashMap::new(); + let mut environments = Vec::with_capacity(selected.len()); + + for env in &selected { + let mut vars = Vec::with_capacity(env.vars.len()); + for v in &env.vars { + let item = match items_by_id.get(&v.item_id) { + Some(it) => it, + // Dangling reference (item deleted after linking) — skip + // cleanly rather than fail the whole send. + None => continue, + }; + let item_name = item.name.clone().unwrap_or_default(); + bundled.entry(item_name.clone()).or_insert_with(|| PlainItem { + item_type: item.item_type.clone(), + name: item_name.clone(), + value: item.value.clone(), + username: item.username.clone(), + password: item.password.clone(), + url: item.url.clone(), + notes: item.notes.clone(), + category: item.categories.clone().and_then(|c| c.into_iter().next()), + command: item.command.clone(), + }); + vars.push(ProjectBundleVar { key: v.key.clone(), item_name }); + } + environments.push(EnvironmentBundle { + name: env.name.clone(), + is_default: env.is_default, + vars, + }); + } + + let bundle = ProjectBundle { + kind: ProjectBundle::KIND.to_string(), + version: ProjectBundle::VERSION, + name: project.name.clone(), + description: project.description.clone(), + template: project.template.clone(), + environments, + items: bundled.into_values().collect(), + }; + + let json_len = serde_json::to_vec(&bundle).map_err(|e| e.to_string())?.len(); + if json_len > MAX_BUNDLE_BYTES { + return Err(format!( + "project bundle too large ({} KiB, max {} KiB) — share fewer environments", + json_len / 1024, + MAX_BUNDLE_BYTES / 1024 + )); + } + + Ok(bundle) +} + +/// Result of a successful project-relay receive. +#[derive(Debug)] +pub struct ReceivedProject { + pub project_id: i64, + pub project_name: String, + pub environment_names: Vec, + pub item_count: usize, +} + +/// Recreates `bundle` as a brand-new project in the receiver's vault, in one +/// all-or-nothing transaction (D7). `name_override` lets the caller rename +/// on a collision retry (D5) instead of guessing a name here. +/// +/// Default-environment promotion: if the sender deselected the default +/// environment, none of the bundled environments has `is_default = true` — +/// the first one in the bundle is promoted so the receiver never ends up +/// with a project that has no default environment. +pub async fn receive_project_bundle( + db: &VaultDb, + vault_key: &[u8; 32], + bundle: ProjectBundle, + name_override: Option, +) -> Result { + let project_name = name_override.unwrap_or_else(|| bundle.name.clone()); + + // Issue #7's name validation is enforced in `save_project` / + // `save_environment`, which this path deliberately bypasses (it writes + // the whole project in one transaction via `insert_received_project`). + // Re-assert it here: every name in the bundle is remote, attacker- + // controllable input, and this is the only layer-1 check on the receive + // path. `fsguard::resolve_within` remains the independent layer 2 at the + // write sites, so a name slipping past here still cannot escape an + // output directory — but an unvalidated name must not reach the DB in + // the first place. + crate::project::validate_project_name(&project_name)?; + for env in &bundle.environments { + crate::project::validate_environment_name(&env.name)?; + } + + let created = now_ts(); + + // Encrypt each unique item with the RECEIVER's key — items never cross + // vaults as ciphertext, only as the decrypted `PlainItem`s already + // carried inside the bundle. + let mut items = Vec::with_capacity(bundle.items.len()); + for plain in &bundle.items { + let (item_type, ciphertext) = + crate::share::build_encrypted_item(plain, vault_key, &created).map_err(|e| e.to_string())?; + items.push(ReceivedProjectItem { + name: plain.name.clone(), + item_type, + ciphertext, + created: created.clone(), + }); + } + + let mut environments: Vec = bundle + .environments + .iter() + .map(|e| ReceivedEnvironment { + name: e.name.clone(), + is_default: e.is_default, + vars: e + .vars + .iter() + .map(|v| ReceivedVar { key: v.key.clone(), item_name: v.item_name.clone() }) + .collect(), + }) + .collect(); + + if !environments.iter().any(|e| e.is_default) { + if let Some(first) = environments.first_mut() { + first.is_default = true; + } + } + + let environment_names: Vec = environments.iter().map(|e| e.name.clone()).collect(); + let item_count = items.len(); + + let inserted = db + .insert_received_project(&project_name, bundle.description.as_deref(), &bundle.template, &items, &environments) + .await?; + + Ok(ReceivedProject { + project_id: inserted.project_id, + project_name, + environment_names, + item_count, + }) +} diff --git a/src-tauri/src/project/relay_commands.rs b/src-tauri/src/project/relay_commands.rs new file mode 100644 index 0000000..c4ef7f1 --- /dev/null +++ b/src-tauri/src/project/relay_commands.rs @@ -0,0 +1,158 @@ +//! Tauri commands for whole-project sharing via the encrypted relay (issue +//! #4). Modelled on `vault::share_commands::{share_relay_send, share_relay_receive}` +//! — same `guard.touch()` calls, same bundled-default fallback for the relay +//! URL/anon key (`DEFAULT_RELAY_URL`/`DEFAULT_RELAY_ANON_KEY`). Note the +//! existing inconsistency with the HTTP handlers (which hard-fail with +//! `NOT_CONFIGURED` instead of falling back to a bundled default) is +//! preserved rather than "fixed" here — that's a separate concern from this +//! feature. + +use tauri::State; +use serde::Serialize; + +use crate::project::relay::{build_project_bundle, receive_project_bundle}; +use crate::share::relay; +use crate::vault::SharedState; + +const DEFAULT_RELAY_URL: &str = match option_env!("CRYPTENV_RELAY_URL") { + Some(v) => v, + None => "", +}; +const DEFAULT_RELAY_ANON_KEY: &str = match option_env!("CRYPTENV_RELAY_ANON_KEY") { + Some(v) => v, + None => "", +}; + +#[derive(Serialize)] +pub struct RelayShareResult { + pub code: String, + pub passphrase: String, + pub project: String, + #[serde(rename = "environmentCount")] + pub environment_count: usize, + #[serde(rename = "itemCount")] + pub item_count: usize, +} + +#[tauri::command] +pub async fn project_relay_send( + project_id: i64, + environment_ids: Vec, + vault_state: State<'_, SharedState>, +) -> Result { + let (supabase_url, anon_key, bundle) = { + let mut guard = vault_state.lock().await; + let k = guard.key.as_ref().ok_or("vault is locked")?; + let vault_key: [u8; 32] = **k; + guard.touch(); + + let supabase_url = guard + .db + .get_setting("relay_supabase_url") + .await? + .unwrap_or_else(|| DEFAULT_RELAY_URL.to_string()); + let anon_key = guard + .db + .get_setting("relay_supabase_anon_key") + .await? + .unwrap_or_else(|| DEFAULT_RELAY_ANON_KEY.to_string()); + + if supabase_url.is_empty() || anon_key.is_empty() { + return Err("Supabase relay not configured. Go to Settings → Internet Sharing to add your Supabase URL and API key.".to_string()); + } + + let bundle = build_project_bundle(&guard.db, &vault_key, project_id, &environment_ids).await?; + (supabase_url, anon_key, bundle) + }; + + let project_name = bundle.name.clone(); + let environment_count = bundle.environments.len(); + let item_count = bundle.items.len(); + + let code = relay::generate_share_code(); + let passphrase = crate::share::crypto::generate_passphrase(); + + let relay_key = relay::derive_relay_key(&code, &passphrase).map_err(|e| e.to_string())?; + let payload = relay::encrypt_project(&bundle, &relay_key).map_err(|e| e.to_string())?; + + let code_clone = code.clone(); + let url_clone = supabase_url.clone(); + let key_clone = anon_key.clone(); + tokio::task::spawn_blocking(move || relay::relay_upload(&url_clone, &key_clone, &code_clone, &payload)) + .await + .map_err(|e| format!("relay send failed: {e}"))? + .map_err(|e| format!("relay send failed: {e}"))?; + + { vault_state.lock().await.touch(); } + + Ok(RelayShareResult { code, passphrase, project: project_name, environment_count, item_count }) +} + +#[derive(Serialize)] +pub struct ProjectReceiveResult { + pub project: String, + pub environments: Vec, + #[serde(rename = "itemCount")] + pub item_count: usize, +} + +#[tauri::command] +pub async fn project_relay_receive( + code: String, + passphrase: String, + project_name_override: Option, + vault_state: State<'_, SharedState>, +) -> Result { + let (vault_key, supabase_url, anon_key) = { + let mut guard = vault_state.lock().await; + let k = guard.key.as_ref().ok_or("vault is locked")?; + let vault_key: [u8; 32] = **k; + guard.touch(); + + let supabase_url = guard + .db + .get_setting("relay_supabase_url") + .await? + .unwrap_or_else(|| DEFAULT_RELAY_URL.to_string()); + let anon_key = guard + .db + .get_setting("relay_supabase_anon_key") + .await? + .unwrap_or_else(|| DEFAULT_RELAY_ANON_KEY.to_string()); + + if supabase_url.is_empty() || anon_key.is_empty() { + return Err("Supabase relay not configured. Go to Settings → Internet Sharing to add your Supabase URL and API key.".to_string()); + } + + (vault_key, supabase_url, anon_key) + }; + + let relay_key = relay::derive_relay_key(&code, &passphrase).map_err(|e| e.to_string())?; + + let code_clone = code.clone(); + let url_clone = supabase_url.clone(); + let key_clone = anon_key.clone(); + let payload = tokio::task::spawn_blocking(move || relay::relay_download(&url_clone, &key_clone, &code_clone)) + .await + .map_err(|e| format!("relay receive failed: {e}"))? + .map_err(|e| format!("relay receive failed: {e}"))?; + + let bundle = relay::decrypt_project(&payload, &relay_key) + .map_err(|e| format!("relay receive failed: could not decrypt payload — {e}"))?; + + // Burn-after-read (best-effort) — same as every other relay receive path. + let url_clone2 = supabase_url.clone(); + let key_clone2 = anon_key.clone(); + let code_clone2 = code.clone(); + let _ = tokio::task::spawn_blocking(move || relay::relay_delete(&url_clone2, &key_clone2, &code_clone2)).await; + + let mut guard = vault_state.lock().await; + guard.touch(); + let result = receive_project_bundle(&guard.db, &vault_key, bundle, project_name_override).await?; + + Ok(ProjectReceiveResult { + project: result.project_name, + environments: result.environment_names, + item_count: result.item_count, + }) +} diff --git a/src-tauri/src/share/mod.rs b/src-tauri/src/share/mod.rs index 97c3f35..998e00d 100644 --- a/src-tauri/src/share/mod.rs +++ b/src-tauri/src/share/mod.rs @@ -629,6 +629,43 @@ async fn run_receive_background( Ok(()) } +/// Serializes and encrypts a single `PlainItem` into vault-item ciphertext: +/// `(item_type, encrypted_data)`, ready for `db::upsert_item` or a batched +/// insert like `db::insert_received_project`. Extracted out of +/// `import_plain_items_into_vault`'s per-item body (issue #4 D7) so the +/// project-relay receive path — which needs ciphertext up front for a single +/// transactional multi-row insert, rather than one `db.upsert_item` call per +/// item — can share the encrypt step instead of duplicating it. `db` never +/// sees the vault key or a `PlainItem`, only the ciphertext this returns. +pub(crate) fn build_encrypted_item( + plain: &PlainItem, + vault_key: &[u8; 32], + created: &str, +) -> Result<(String, String), ShareError> { + let vault_item = crate::vault::VaultItem { + id: 0, + item_type: plain.item_type.clone(), + name: Some(plain.name.clone()), + value: plain.value.clone(), + username: plain.username.clone(), + password: plain.password.clone(), + url: plain.url.clone(), + notes: plain.notes.clone(), + title: None, + description: None, + command: plain.command.clone(), + shell: None, + content: None, + categories: Some(plain.category.iter().cloned().collect()), + created: created.to_string(), + is_global: None, + }; + + let json = serde_json::to_vec(&vault_item).map_err(|e| ShareError::Protocol(e.to_string()))?; + let encrypted = crate::crypto::encrypt(vault_key, &json).map_err(ShareError::Vault)?; + Ok((vault_item.item_type, encrypted)) +} + /// Result of [`import_plain_items_into_vault`]. pub struct ImportOutcome { /// Names of every item actually imported into the vault (regardless of @@ -685,32 +722,10 @@ pub(crate) async fn import_plain_items_into_vault( let mut skipped_keys = Vec::new(); for plain in items { - let vault_item = crate::vault::VaultItem { - id: 0, - item_type: plain.item_type.clone(), - name: Some(plain.name.clone()), - value: plain.value.clone(), - username: plain.username.clone(), - password: plain.password.clone(), - url: plain.url.clone(), - notes: plain.notes.clone(), - title: None, - description: None, - command: plain.command.clone(), - shell: None, - content: None, - categories: Some(plain.category.iter().cloned().collect()), - created: now_ts.clone(), - is_global: None, - }; - - let json = serde_json::to_vec(&vault_item) - .map_err(|e| ShareError::Protocol(e.to_string()))?; - let encrypted = crate::crypto::encrypt(vault_key, &json) - .map_err(|e| ShareError::Vault(e))?; + let (item_type, encrypted) = build_encrypted_item(plain, vault_key, &now_ts)?; let new_id = db - .upsert_item(0, &vault_item.item_type, &encrypted, &vault_item.created, false) + .upsert_item(0, &item_type, &encrypted, &now_ts, false) .await .map_err(|e| ShareError::Vault(e))?; diff --git a/src-tauri/src/share/relay.rs b/src-tauri/src/share/relay.rs index e09cf1d..5021ea6 100644 --- a/src-tauri/src/share/relay.rs +++ b/src-tauri/src/share/relay.rs @@ -54,58 +54,89 @@ pub fn decrypt_payload(payload: &str, key: &[u8; 32]) -> Result, serde_json::from_slice(&plaintext).map_err(|e| ShareError::Protocol(e.to_string())) } -// ─── Complete-workspace bundle (template + values) ──────────────────────────── -// A workspace bundle carries the workspace definition AND the decrypted values of -// every referenced vault item, so a teammate can reconstruct a ready-to-inject -// workspace in one step. It rides on the same relay encryption as plain items. - -/// One variable in a shared workspace. Either references a bundled item (by name) -/// or carries an inline literal value — never both. -#[derive(Serialize, Deserialize, Clone)] -pub struct WorkspaceBundleVar { +// ─── Project bundle (structure + values for N environments at once) ────────── +// A project bundle carries an entire project's definition — its environments +// and the decrypted values of every item they reference — so a teammate can +// reconstruct a ready-to-inject multi-environment project in one step. Items +// are hoisted to the bundle root and deduped by name, referenced from each +// environment's vars by name (see docs/plans/issue-4 D1): this is what lets +// "the same item linked into 3 environments" be told apart from "3 items +// that happen to share a name" on receive, which the old per-environment +// `WorkspaceBundle` shape (removed) could not distinguish. + +/// One variable in a shared environment — always references a bundled item +/// by name. Unlike the legacy workspace format, there is no inline literal: +/// `environment_vars.item_id` is mandatory post-migration, so every var this +/// bundle can even represent already resolves to a real item (D3). +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ProjectBundleVar { pub key: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub item_name: Option, - #[serde(skip_serializing_if = "Option::is_none")] - pub literal: Option, + pub item_name: String, +} + +/// One environment's shape, values-free of anything machine-specific. +/// Deliberately has no `paths` field (D6) — absolute filesystem paths from +/// the sender's machine are meaningless (and identity-leaking) on the +/// receiver's, so the field does not exist in the wire type rather than +/// being included-but-ignored. +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct EnvironmentBundle { + pub name: String, + pub is_default: bool, + pub vars: Vec, } -/// A complete, self-contained workspace ready to import. -#[derive(Serialize, Deserialize, Clone)] -pub struct WorkspaceBundle { - /// Discriminator so a workspace payload is never mistaken for a bare item list. +/// A complete, self-contained project ready to import: N environments plus +/// the deduped set of items they reference. +#[derive(Serialize, Deserialize, Clone, Debug)] +pub struct ProjectBundle { + /// Discriminator so a project payload is never mistaken for a bare item list. pub kind: String, + /// Format version, checked on decrypt (D2) — bumping it is free now and + /// impossible to retrofit later, so it's checked from day one even + /// though only version 1 currently exists. + pub version: u32, pub name: String, #[serde(skip_serializing_if = "Option::is_none")] pub description: Option, pub template: String, - pub vars: Vec, - /// Decrypted values for the item-backed vars. Matched to vars by `name`. + pub environments: Vec, + /// Decrypted values, deduped by name. Matched to vars by `item_name`. pub items: Vec, } -impl WorkspaceBundle { - pub const KIND: &'static str = "workspace"; +impl ProjectBundle { + pub const KIND: &'static str = "project"; + pub const VERSION: u32 = 1; } -pub fn encrypt_workspace(bundle: &WorkspaceBundle, key: &[u8; 32]) -> Result { +pub fn encrypt_project(bundle: &ProjectBundle, key: &[u8; 32]) -> Result { let json = serde_json::to_vec(bundle).map_err(|e| ShareError::Protocol(e.to_string()))?; let ciphertext = encrypt_message(key, &json); Ok(B64.encode(&ciphertext)) } -pub fn decrypt_workspace(payload: &str, key: &[u8; 32]) -> Result { +/// Decrypts and validates a project bundle: rejects a wrong `kind` (e.g. a +/// payload produced by `encrypt_items`) and an unknown `version` before +/// returning, so a format change never has to repeat this discriminator dance. +pub fn decrypt_project(payload: &str, key: &[u8; 32]) -> Result { let ciphertext = B64 .decode(payload) .map_err(|e| ShareError::Protocol(format!("base64 decode: {e}")))?; let plaintext = decrypt_message(key, &ciphertext)?; - let bundle: WorkspaceBundle = + let bundle: ProjectBundle = serde_json::from_slice(&plaintext).map_err(|e| ShareError::Protocol(e.to_string()))?; - if bundle.kind != WorkspaceBundle::KIND { + if bundle.kind != ProjectBundle::KIND { return Err(ShareError::Protocol( - "this code is not a workspace package (use the items receive flow instead)".into(), + "this code is not a project package (use the items receive flow instead)".into(), )); } + if bundle.version != ProjectBundle::VERSION { + return Err(ShareError::Protocol(format!( + "this package was created by a newer version of CryptEnv (format v{}); update to receive it", + bundle.version + ))); + } Ok(bundle) } @@ -267,3 +298,132 @@ fn days_to_ymd(days: u64) -> (u64, u64, u64) { fn is_leap(y: i64) -> bool { (y % 4 == 0 && y % 100 != 0) || y % 400 == 0 } + +#[cfg(test)] +mod tests { + use super::*; + + fn sample_bundle() -> ProjectBundle { + ProjectBundle { + kind: ProjectBundle::KIND.to_string(), + version: ProjectBundle::VERSION, + name: "MyApp".to_string(), + description: Some("a sample project".to_string()), + template: "node".to_string(), + environments: vec![ + EnvironmentBundle { + name: "local".to_string(), + is_default: true, + vars: vec![ + ProjectBundleVar { key: "DB_HOST".to_string(), item_name: "db-host".to_string() }, + ProjectBundleVar { key: "DB_PASSWORD".to_string(), item_name: "db-password".to_string() }, + ], + }, + EnvironmentBundle { + name: "production".to_string(), + is_default: false, + vars: vec![ProjectBundleVar { + key: "DB_HOST".to_string(), + item_name: "db-host".to_string(), + }], + }, + ], + items: vec![ + PlainItem { + item_type: "secret".to_string(), + name: "db-host".to_string(), + value: Some("localhost".to_string()), + username: None, + password: None, + url: None, + notes: None, + category: None, + command: None, + }, + PlainItem { + item_type: "secret".to_string(), + name: "db-password".to_string(), + value: Some("hunter2".to_string()), + username: None, + password: None, + url: None, + notes: None, + category: None, + command: None, + }, + ], + } + } + + #[test] + fn project_bundle_roundtrip_preserves_structure() { + let bundle = sample_bundle(); + let key = derive_relay_key("TEST-CODE", "correct horse battery staple").unwrap(); + + let payload = encrypt_project(&bundle, &key).unwrap(); + let decrypted = decrypt_project(&payload, &key).unwrap(); + + assert_eq!(decrypted.environments.len(), bundle.environments.len()); + for (a, b) in decrypted.environments.iter().zip(bundle.environments.iter()) { + assert_eq!(a.name, b.name); + assert_eq!(a.is_default, b.is_default); + let a_keys: Vec<&str> = a.vars.iter().map(|v| v.key.as_str()).collect(); + let b_keys: Vec<&str> = b.vars.iter().map(|v| v.key.as_str()).collect(); + assert_eq!(a_keys, b_keys); + } + let mut a_names: Vec<&str> = decrypted.items.iter().map(|i| i.name.as_str()).collect(); + let mut b_names: Vec<&str> = bundle.items.iter().map(|i| i.name.as_str()).collect(); + a_names.sort(); + b_names.sort(); + assert_eq!(a_names, b_names); + } + + #[test] + fn decrypt_project_rejects_items_payload() { + let key = derive_relay_key("TEST-CODE", "correct horse battery staple").unwrap(); + let items = vec![PlainItem { + item_type: "secret".to_string(), + name: "loose-item".to_string(), + value: Some("x".to_string()), + username: None, + password: None, + url: None, + notes: None, + category: None, + command: None, + }]; + let payload = encrypt_items(&items, &key).unwrap(); + + let result = decrypt_project(&payload, &key); + match result { + Err(ShareError::Protocol(_)) => {} + other => panic!("expected ShareError::Protocol, got {other:?}"), + } + } + + #[test] + fn decrypt_project_rejects_unknown_version() { + let key = derive_relay_key("TEST-CODE", "correct horse battery staple").unwrap(); + let mut bundle = sample_bundle(); + bundle.version = 99; + let payload = encrypt_project(&bundle, &key).unwrap(); + + let err = decrypt_project(&payload, &key).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("v99"), "message should name the unsupported version, got: {msg}"); + } + + #[test] + fn decrypt_project_rejects_wrong_passphrase() { + let bundle = sample_bundle(); + let key = derive_relay_key("TEST-CODE", "correct horse battery staple").unwrap(); + let payload = encrypt_project(&bundle, &key).unwrap(); + + let wrong_key = derive_relay_key("TEST-CODE", "totally different passphrase").unwrap(); + let result = decrypt_project(&payload, &wrong_key); + match result { + Err(ShareError::Crypto(_)) => {} + other => panic!("expected ShareError::Crypto, got {other:?}"), + } + } +} diff --git a/src-tauri/src/vault/mod.rs b/src-tauri/src/vault/mod.rs index c595a18..723d810 100644 --- a/src-tauri/src/vault/mod.rs +++ b/src-tauri/src/vault/mod.rs @@ -8,7 +8,7 @@ use zeroize::Zeroizing; use crate::biometric; use crate::crypto::{self, CryptoKey}; -use crate::db::{DbCategory, VaultDb}; +use crate::db::{DbCategory, LinkMode, LinkOutcome, VaultDb}; pub mod import; pub mod share_commands; @@ -301,6 +301,102 @@ pub async fn create_project_item( Ok(new_id) } +/// `db`'s conflict classification, re-exported so callers outside this crate +/// (namely `api`) match on the `vault` type rather than importing from `db` +/// directly — `vault` is the layer that owns the create-or-update decision, +/// `db` only executes it. +pub use crate::db::ConflictReason; + +/// Result of `create_or_update_env_item` — mirrors `db::LinkOutcome` but +/// carries the full (decrypted, redaction-pending) `VaultItem` on success +/// instead of bare ids, since the HTTP response needs the whole shape. +pub enum UpsertOutcome { + Created(VaultItem), + Updated(VaultItem), + Conflict { item_id: i64, reason: ConflictReason }, +} + +/// Orchestrates `POST /items`'s create-or-update-on-collision behaviour +/// (issue #9): inspect the current state of `env_key` in `environment_id`, +/// decide free / exclusive / shared, encrypt (preserving the existing row's +/// `created` on the update branch so the ciphertext and the column agree), +/// then hand the encrypted blob to `db::create_or_link_item`'s single +/// transaction. This is the only layer that holds the vault key on this +/// path — `db` never sees a `CryptoKey`. +pub async fn create_or_update_env_item( + db: &VaultDb, + key: &CryptoKey, + item: &VaultItem, + project_id: i64, + environment_id: i64, + env_key: &str, + mode: LinkMode, +) -> Result { + let conflict = db.inspect_env_key(environment_id, env_key).await?; + + // Short-circuit without touching the key: no need to encrypt anything to + // return a conflict. + if let Some(c) = &conflict { + if mode == LinkMode::Error { + return Ok(UpsertOutcome::Conflict { + item_id: c.item_id, + reason: ConflictReason::KeyExists, + }); + } + if mode == LinkMode::Update { + let exclusive = !c.is_global && c.link_count == 1 && c.owner_ids == [project_id]; + if !exclusive { + return Ok(UpsertOutcome::Conflict { + item_id: c.item_id, + reason: ConflictReason::Shared, + }); + } + } + } + + // Build the item to persist. On the `Update` branch, stamp the existing + // row's `created` onto the blob so it matches `items.created`, which the + // update branch of `create_or_link_item` does not rewrite. On create / + // replace, keep the caller's `created` (already defaulted by the handler). + let mut to_persist = item.clone(); + if mode == LinkMode::Update { + if let Some(c) = &conflict { + to_persist.created = c.created.clone(); + } + } + + let encrypted = encrypt_item(key, &to_persist)?; + + let outcome = db + .create_or_link_item( + environment_id, + project_id, + env_key, + &to_persist.item_type, + &encrypted, + &to_persist.created, + mode, + conflict.map(|c| c.item_id), + ) + .await?; + + Ok(match outcome { + LinkOutcome::Created { item_id, is_global } => { + let mut saved = to_persist; + saved.id = item_id; + saved.is_global = Some(is_global); + UpsertOutcome::Created(saved) + } + LinkOutcome::Updated { item_id, is_global } => { + let mut saved = to_persist; + saved.id = item_id; + saved.is_global = Some(is_global); + UpsertOutcome::Updated(saved) + } + LinkOutcome::Conflict { item_id, reason } => UpsertOutcome::Conflict { item_id, reason }, + }) +} + /// Creates a new item and atomically grants ownership to `project_id` — the /// "add a typed variable inside a project" primitive. New items are never /// global by default (the caller must explicitly toggle that afterwards). @@ -418,6 +514,53 @@ pub async fn vault_get_item_owners( .collect()) } +// ─── Issue #9: orphan report / prune (report by default, delete only on +// explicit confirmation — see plan §3.6) ─────────────────────────────────── + +/// Decrypts only the ids `db::list_orphan_item_ids` returns and redacts them, +/// so the caller can judge what it is about to delete without any secret +/// value leaving this process. +pub async fn list_orphan_items(db: &VaultDb, key: &CryptoKey) -> Result, String> { + let ids = db.list_orphan_item_ids().await?; + if ids.is_empty() { + return Ok(vec![]); + } + let raw = db.list_items().await?; + Ok(raw + .into_iter() + .filter(|(id, ..)| ids.contains(id)) + .filter_map(|(id, _, data, _, is_global)| decrypt_item(key, id, &data, is_global).ok()) + .collect()) +} + +/// Deletes the given ids via `db::delete_items_cascade`, which re-checks the +/// orphan predicate per id inside its own transaction — a concurrently +/// re-linked or globalized item is left alone rather than destroyed. +pub async fn prune_orphan_items(db: &VaultDb, ids: &[i64]) -> Result<(), String> { + db.delete_items_cascade(ids).await +} + +#[tauri::command] +pub async fn vault_list_orphan_items( + state: State<'_, SharedState>, +) -> Result, String> { + let mut s = state.lock().await; + let key = s.key.as_ref().ok_or("vault is locked")?.clone(); + s.touch(); + list_orphan_items(&s.db, &key).await +} + +#[tauri::command] +pub async fn vault_prune_orphan_items( + ids: Vec, + state: State<'_, SharedState>, +) -> Result<(), String> { + let mut s = state.lock().await; + s.key.as_ref().ok_or("vault is locked")?; + s.touch(); + prune_orphan_items(&s.db, &ids).await +} + #[tauri::command] pub async fn vault_delete_item(id: i64, state: State<'_, SharedState>) -> Result<(), String> { let mut s = state.lock().await; diff --git a/src-tauri/tests/path_containment.rs b/src-tauri/tests/path_containment.rs new file mode 100644 index 0000000..8cc69e6 --- /dev/null +++ b/src-tauri/tests/path_containment.rs @@ -0,0 +1,346 @@ +// Integration tests for issue #7 — path traversal via environment name +// escapes `output_dir` on `/fill`, `/environments/{}/example` and inject. +// See docs/plans/issue-7-path-traversal-environment-name.md. +// +// Issue #11's HTTP test harness (`crate::test_support`, `TestVault`) has not +// landed on this branch, so the HTTP-level cases from the plan's §5 (T3's +// `/fill` secret non-exfiltration scan and T8's error-shape assertions) are +// not implemented here. Per the plan's own stated fallback: the non-HTTP +// cases below still cover objectives 1, 2, 4 and 6. `project::inject_environment` +// stands in as the one non-HTTP-reachable sink (`api::handle_fill` and +// `api::handle_environment_example` are private to the `api` module and only +// reachable through the axum router); all three sinks share the exact same +// `fsguard::resolve_within` call, which is exercised directly below. +// +// Run with: cargo test --test path_containment + +use crypt_env_lib::db::VaultDb; +use crypt_env_lib::fsguard; +use crypt_env_lib::project::{self, EnvironmentInput, EnvironmentVar}; +use std::fs; +use tempfile::tempdir; + +/// At least 17 malicious names (plan §5/T1). Covers `../` runs, `..\` runs, +/// bare `..`/`.`, POSIX/Windows absolute, UNC, verbatim prefix, drive- +/// relative, NUL byte, percent-encoded and overlong-UTF-8-looking, Unicode +/// look-alikes, Windows reserved device names, trailing dot/space, and NTFS +/// alternate-data-stream. Over-length and empty/whitespace are covered +/// separately below (not naturally `&'static str` table entries). +const MALICIOUS: &[&str] = &[ + "../../../tmp/pwned", // POSIX traversal (the issue's repro) + "..\\..\\..\\Windows\\Temp\\pwned", // Windows traversal + "..", // bare parent + ".", // bare current + "/etc/cron.d/pwned", // POSIX absolute + "C:\\Windows\\Temp\\pwned", // Windows absolute + "C:pwned", // Windows drive-relative + "\\\\wsl.localhost\\Ubuntu\\home\\u\\pwned", // UNC + "\\\\?\\C:\\pwned", // verbatim prefix + "prod\0evil", // NUL byte + "%2e%2e%2fpwned", // percent-encoded + "..%c0%af..%c0%afpwned", // overlong-UTF-8-looking + "‥/pwned", // Unicode look-alike (U+2025) + "../pwned", // Unicode look-alike (fullwidth) + "CON", // reserved device name + "NUL", // reserved device name + "COM1", // reserved device name + "LPT1", // reserved device name + "prod.", // trailing dot + "prod ", // trailing space + "env:stream", // NTFS ADS +]; + +fn overlength_name() -> String { + "a".repeat(65) +} + +// ─── T1 — the malicious table (objectives 1 and 2) ──────────────────────────── + +#[test] +fn t1_malicious_table_rejected_by_both_layers() { + let dir = tempdir().unwrap(); + let base = dir.path().to_str().unwrap(); + + for name in MALICIOUS { + assert!( + project::validate_environment_name(name).is_err(), + "layer 1 (validate_environment_name) should reject {name:?}" + ); + + // Note on the percent-encoded / overlong-UTF-8-looking / Unicode + // look-alike entries: these must be treated as literal filenames, + // never decoded. Both outcomes below are acceptable per the plan — + // rejection, or a path that resolves but stays a direct child of + // the canonicalized base. Silently *decoding* them into a real `..` + // is the only forbidden outcome, and that would surface as an + // escaped/non-child path, which the assertions below would catch. + match fsguard::resolve_within(base, &format!(".env.{name}")) { + Err(_) => {} + Ok(p) => { + let canon = fs::canonicalize(base).unwrap(); + assert!(p.starts_with(&canon), "escaped base for {name:?}: {p:?}"); + assert_eq!( + p.parent(), + Some(canon.as_path()), + "not a direct child of base for {name:?}: {p:?}" + ); + } + } + } + + // Over-length (>64), tested separately since `String` can't live in a + // `&'static [&str]` table. + let long = overlength_name(); + assert!(project::validate_environment_name(&long).is_err(), "layer 1 should reject over-length names"); + assert!( + fsguard::resolve_within(base, &format!(".env.{long}")).is_err() + || fsguard::resolve_within(base, &format!(".env.{long}")) + .unwrap() + .starts_with(fs::canonicalize(base).unwrap()), + "layer 2 must at least contain an over-length name if it doesn't reject it" + ); + + // Empty / whitespace-only. + for empty in ["", " "] { + assert!(project::validate_environment_name(empty).is_err(), "layer 1 should reject {empty:?}"); + assert!(fsguard::resolve_within(base, empty).is_err(), "layer 2 should reject {empty:?}"); + } +} + +// ─── T2 analog — filesystem invariant (objectives 1 and 4) ──────────────────── +// +// The plan's T2 runs the table through all three HTTP-reachable sinks; here +// we run it through `fsguard::resolve_within` directly (the shared primitive +// behind all three) plus the one non-HTTP sink, `project::inject_environment`. + +#[tokio::test] +async fn t2_filesystem_invariant_no_amplification() { + let root = tempdir().unwrap(); + let base_dir = root.path().join("base"); + let sibling_dir = root.path().join("sibling"); + fs::create_dir_all(&base_dir).unwrap(); + fs::create_dir_all(&sibling_dir).unwrap(); + let canary_path = sibling_dir.join("canary.txt"); + fs::write(&canary_path, b"canary-untouched").unwrap(); + + let base = base_dir.to_str().unwrap(); + for name in MALICIOUS { + let _ = fsguard::resolve_within(base, &format!(".env.{name}")); + } + + // Sibling directory (this issue's `/tmp/pwned` stand-in) byte-identical. + assert_eq!( + fs::read(&canary_path).unwrap(), + b"canary-untouched", + "a sibling of the base must never be touched" + ); + + // No directory named `.env...` (or any other attacker-named directory) + // was created under base — kills the `create_dir_all` amplification + // specifically (objective 4). Only the base itself may exist; + // `fsguard::resolve_within` never creates anything beyond it. + let created_dirs: Vec = fs::read_dir(&base_dir) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.path().is_dir()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + assert!( + created_dirs.is_empty(), + "no directories should ever be created under base by a rejected name, found: {created_dirs:?}" + ); +} + +#[tokio::test] +async fn t2_inject_environment_sink_no_amplification() { + let db_dir = tempdir().unwrap(); + let db_path = db_dir.path().join("vault.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + let project_id = db.upsert_project(0, "t2-project", None, "generic").await.unwrap(); + + let root = tempdir().unwrap(); + let base_dir = root.path().join("base"); + fs::create_dir_all(&base_dir).unwrap(); + let vault_key = [0u8; 32]; + + for name in MALICIOUS { + // Bypass layer 1 (`db::upsert_environment` directly) to exercise + // layer 2 in isolation, same as a legacy row would. `inject_environment` + // builds the filename as `.env.` (same convention as + // `/fill` and `/example`), so the stored name is used as-is here — + // matching how T1 exercises `fsguard::resolve_within` directly. + let env_id = match db.upsert_environment(0, project_id, name, false).await { + Ok(id) => id, + Err(_) => continue, // e.g. embedded NUL byte rejected by the DB driver itself + }; + let _ = project::inject_environment( + &db, + &vault_key, + env_id, + None, + Some(base_dir.to_str().unwrap().to_string()), + false, + ) + .await; + } + + let created_dirs: Vec = fs::read_dir(&base_dir) + .unwrap() + .filter_map(|e| e.ok()) + .filter(|e| e.path().is_dir()) + .map(|e| e.file_name().to_string_lossy().into_owned()) + .collect(); + assert!( + created_dirs.is_empty(), + "inject_environment must never create a directory under base via a hostile name, found: {created_dirs:?}" + ); +} + +// ─── T4 — choke-point coverage (objective 6) ─────────────────────────────────── +// +// Calls `project::save_environment` directly with no HTTP involved — this is +// the test that would have caught the original bug, where the HTTP handler +// was the only gate and the Tauri command path had none. + +#[tokio::test] +async fn t4_choke_point_rejects_at_save_environment_directly() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("vault.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + let project_id = db.upsert_project(0, "t4-project", None, "generic").await.unwrap(); + + let input = EnvironmentInput { + id: 0, + project_id, + name: "../../../tmp/pwned".to_string(), + is_default: false, + paths: vec![], + vars: Vec::::new(), + }; + + let result = project::save_environment(&db, input).await; + assert!(result.is_err(), "save_environment must reject a traversal name with no HTTP involved"); + assert!( + result.unwrap_err().starts_with("name: "), + "rejection must be a layer-1 validation error, not a downstream failure" + ); +} + +// ─── T5 — layer independence (objective 2) ───────────────────────────────────── +// +// Disabling either layer must leave the other's tests green. Implemented as +// two test fns that call each layer directly and in isolation, rather than +// by mutating production code. + +#[test] +fn t5_layer2_alone_still_contains_every_malicious_name() { + // Simulates "layer 1 disabled": call `fsguard::resolve_within` directly + // (as a legacy row that bypassed `validate_environment_name` would), + // without ever consulting layer 1. + let dir = tempdir().unwrap(); + let base = dir.path().to_str().unwrap(); + for name in MALICIOUS { + match fsguard::resolve_within(base, &format!(".env.{name}")) { + Err(_) => {} + Ok(p) => { + let canon = fs::canonicalize(base).unwrap(); + assert!(p.starts_with(&canon), "layer 2 alone failed to contain {name:?}: {p:?}"); + } + } + } +} + +#[test] +fn t5_layer1_alone_still_rejects_every_malicious_name() { + // Simulates "layer 2 disabled": call `validate_environment_name` in + // isolation, without ever consulting `fsguard`. + for name in MALICIOUS { + assert!( + project::validate_environment_name(name).is_err(), + "layer 1 alone failed to reject {name:?}" + ); + } +} + +// ─── T6 — positive path, no over-rejection (objective 5) ────────────────────── + +#[test] +fn t6_valid_names_are_accepted_and_resolve_as_expected() { + for name in ["production", "local", "staging-2", "v1.2", "a"] { + assert!(project::validate_environment_name(name).is_ok(), "{name:?} should be a valid environment name"); + } + let sixty_four = "a".repeat(64); + assert!(project::validate_environment_name(&sixty_four).is_ok(), "64 chars should be accepted"); + + let dir = tempdir().unwrap(); + let base = dir.path().to_str().unwrap(); + let resolved = fsguard::resolve_within(base, ".env.production").unwrap(); + let expected = fs::canonicalize(base).unwrap().join(".env.production"); + assert_eq!(resolved, expected, "objective 5: resolved path must be the canonicalized base joined with the filename"); +} + +// ─── T7 — legacy-name containment (D2) ───────────────────────────────────────── +// +// Inserts a row with a hostile name directly through `db::upsert_environment`, +// bypassing validation — simulating a pre-fix vault. Asserts `inject_environment` +// (the one non-HTTP sink) rejects it at layer 2, and that the environment +// remains listable and deletable — no vault bricking, per plan §4/D2. + +#[tokio::test] +async fn t7_legacy_hostile_name_contained_and_vault_stays_usable() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("vault.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + let project_id = db.upsert_project(0, "legacy-project", None, "generic").await.unwrap(); + + // Bypasses `project::validate_environment_name` entirely. + let env_id = db + .upsert_environment(0, project_id, "../../../tmp/pwned", false) + .await + .unwrap(); + + let output_root = tempdir().unwrap(); + let base_dir = output_root.path().join("base"); + fs::create_dir_all(&base_dir).unwrap(); + let vault_key = [0u8; 32]; + + let result = project::inject_environment( + &db, + &vault_key, + env_id, + None, + Some(base_dir.to_str().unwrap().to_string()), + false, + ) + .await; + assert!(result.is_err(), "a legacy hostile name must still be rejected at layer 2"); + + // No amplification directory under base. + let has_amplified_dir = fs::read_dir(&base_dir) + .unwrap() + .filter_map(|e| e.ok()) + .any(|e| e.path().is_dir()); + assert!(!has_amplified_dir, "create_dir_all must never see the interpolated legacy name"); + + // Vault stays usable: the environment is still listable and deletable. + let envs = db.list_environments(project_id).await.unwrap(); + assert!(envs.iter().any(|e| e.id == env_id), "legacy environment must remain listable"); + db.delete_environment(env_id).await.unwrap(); + let envs_after = db.list_environments(project_id).await.unwrap(); + assert!(!envs_after.iter().any(|e| e.id == env_id), "legacy environment must remain deletable"); +} + +// ─── Project-name validation (D2) ────────────────────────────────────────────── +// +// Not one of the plan's numbered T-cases, but covers `validate_project_name` +// directly since `save_project` shares the same choke-point pattern. + +#[test] +fn project_name_allows_human_labels_rejects_hostile_ones() { + for name in ["My App", "Café Backend", "a", &"a".repeat(128)] { + assert!(project::validate_project_name(name).is_ok(), "{name:?} should be a valid project name"); + } + for name in ["../../../tmp/pwned", "CON", "a/b", "a\\b", "..", ".", "trailing.", "trailing ", &"a".repeat(129)] { + assert!(project::validate_project_name(name).is_err(), "{name:?} should be rejected"); + } +} diff --git a/src-tauri/tests/project_relay.rs b/src-tauri/tests/project_relay.rs new file mode 100644 index 0000000..5cf9468 --- /dev/null +++ b/src-tauri/tests/project_relay.rs @@ -0,0 +1,344 @@ +// Integration tests for the whole-project relay feature (issue #4), exercised +// directly against the `db`/`project::relay` layers (no HTTP harness): issue +// #11's `test_support` fixture (`TestVault`, `seed_project`, `seed_item`, +// `link_var`) is not available in this worktree yet, so these tests build +// their own minimal local fixtures on top of `VaultDb` — the same pattern +// used by `tests/vault_integration.rs`. Once #11's harness lands, these can +// be rebased onto it; the assertions themselves (row counts, dedup, no +// leaked paths) are the ones the plan's cases 5-11 call for. +// +// Transport (relay upload/download) is not exercised here — the bundle is +// built and handed directly to the receive-side function, per the plan's +// "transport stubbed" instruction for this test file. + +use crypt_env_lib::db::VaultDb; +use crypt_env_lib::project::relay as project_relay; +use crypt_env_lib::share::package::PlainItem; +use crypt_env_lib::share::relay::{EnvironmentBundle, ProjectBundle, ProjectBundleVar}; +use crypt_env_lib::vault::VaultItem; +use tempfile::tempdir; + +const SENDER_KEY: [u8; 32] = [7u8; 32]; +const RECEIVER_KEY: [u8; 32] = [9u8; 32]; + +async fn open_db(dir: &tempfile::TempDir) -> VaultDb { + let path = dir.path().join("vault.db").to_str().unwrap().to_string(); + VaultDb::open(&path).await.unwrap() +} + +/// Encrypts and inserts a "secret" item with `name`/`value`, returning its id. +async fn seed_item(db: &VaultDb, key: &[u8; 32], name: &str, value: &str) -> i64 { + let item = VaultItem { + id: 0, + item_type: "secret".to_string(), + name: Some(name.to_string()), + value: Some(value.to_string()), + url: None, + username: None, + password: None, + title: None, + description: None, + command: None, + shell: None, + categories: None, + notes: None, + content: None, + created: "1000".to_string(), + is_global: Some(false), + }; + let json = serde_json::to_vec(&item).unwrap(); + let encrypted = crypt_env_lib::crypto::encrypt(key, &json).unwrap(); + db.upsert_item(0, "secret", &encrypted, "1000", false).await.unwrap() +} + +/// Grants `project_id` ownership of `item_id` and links it into +/// `environment_id` under `key` — mirrors what `save_environment` / +/// `import_plain_items_into_vault` do together in production code. +async fn link_var(db: &VaultDb, environment_id: i64, project_id: i64, item_id: i64, key: &str) { + db.add_item_owner(item_id, project_id).await.unwrap(); + db.upsert_environment_var(environment_id, key, item_id).await.unwrap(); +} + +/// Decrypts every item in `db` and returns (item_id, name, is_global). +async fn decrypt_all(db: &VaultDb, key: &[u8; 32]) -> Vec<(i64, String, bool)> { + let raw = db.list_items().await.unwrap(); + raw.into_iter() + .map(|(id, _item_type, data, _created, is_global)| { + let plaintext = crypt_env_lib::crypto::decrypt(key, &data).unwrap(); + let item: VaultItem = serde_json::from_slice(&plaintext).unwrap(); + (id, item.name.unwrap_or_default(), is_global) + }) + .collect() +} + +/// A 3-environment project (local default, staging, production) with 5 +/// distinct items, 2 of which (S1, S2) are linked into all 3 environments — +/// the fixture named in the plan's case 5. Returns (db, project_id, [local, +/// staging, production]). +async fn seed_three_env_project(dir: &tempfile::TempDir) -> (VaultDb, i64, [i64; 3]) { + let db = open_db(dir).await; + let project_id = db.upsert_project(0, "MyApp", None, "generic").await.unwrap(); + let local = db.upsert_environment(0, project_id, "local", true).await.unwrap(); + let staging = db.upsert_environment(0, project_id, "staging", false).await.unwrap(); + let production = db.upsert_environment(0, project_id, "production", false).await.unwrap(); + + let s1 = seed_item(&db, &SENDER_KEY, "S1", "shared-1").await; + let s2 = seed_item(&db, &SENDER_KEY, "S2", "shared-2").await; + let it3 = seed_item(&db, &SENDER_KEY, "IT3", "v3").await; + let it4 = seed_item(&db, &SENDER_KEY, "IT4", "v4").await; + let it5 = seed_item(&db, &SENDER_KEY, "IT5", "v5").await; + + // local: S1, S2, IT3, IT4 (4) + link_var(&db, local, project_id, s1, "S1").await; + link_var(&db, local, project_id, s2, "S2").await; + link_var(&db, local, project_id, it3, "IT3").await; + link_var(&db, local, project_id, it4, "IT4").await; + + // staging: S1, S2, IT3, IT5 (4) + link_var(&db, staging, project_id, s1, "S1").await; + link_var(&db, staging, project_id, s2, "S2").await; + link_var(&db, staging, project_id, it3, "IT3").await; + link_var(&db, staging, project_id, it5, "IT5").await; + + // production: S1, S2, IT5 (3) + link_var(&db, production, project_id, s1, "S1").await; + link_var(&db, production, project_id, s2, "S2").await; + link_var(&db, production, project_id, it5, "IT5").await; + + // total vars: 4 + 4 + 3 = 11, distinct items: S1, S2, IT3, IT4, IT5 = 5 + (db, project_id, [local, staging, production]) +} + +#[tokio::test] +async fn share_three_env_project_dedups_reused_items() { + let sender_dir = tempdir().unwrap(); + let (db, project_id, envs) = seed_three_env_project(&sender_dir).await; + + let bundle = project_relay::build_project_bundle(&db, &SENDER_KEY, project_id, &envs) + .await + .expect("build_project_bundle should succeed"); + + assert_eq!(bundle.items.len(), 5, "items must be deduped across environments, not 11"); + let total_vars: usize = bundle.environments.iter().map(|e| e.vars.len()).sum(); + assert_eq!(total_vars, 11); + + let receiver_dir = tempdir().unwrap(); + let receiver_db = open_db(&receiver_dir).await; + let result = project_relay::receive_project_bundle(&receiver_db, &RECEIVER_KEY, bundle, None) + .await + .expect("receive_project_bundle should succeed into a fresh vault"); + + assert_eq!(result.environment_names.len(), 3); + assert_eq!(result.item_count, 5); + + let items = receiver_db.list_items().await.unwrap(); + assert_eq!(items.len(), 5, "exactly 5 new items rows"); + + let projects = receiver_db.list_projects().await.unwrap(); + assert_eq!(projects.len(), 1); + let new_project_id = projects[0].id; + + let received_envs = receiver_db.list_environments(new_project_id).await.unwrap(); + assert_eq!(received_envs.len(), 3, "3 environments rows"); + + let mut total_env_var_rows = 0; + for e in &received_envs { + total_env_var_rows += receiver_db.get_environment_vars(e.id).await.unwrap().len(); + } + assert_eq!(total_env_var_rows, 11, "11 environment_vars rows"); + + let owned = receiver_db.list_owned_item_ids(new_project_id).await.unwrap(); + assert_eq!(owned.len(), 5, "5 item_projects rows, all pointing at the new project"); + + // The two shared items appear as one row each -- 0 duplicated ciphertext rows. + let decrypted = decrypt_all(&receiver_db, &RECEIVER_KEY).await; + let count_named = |n: &str| decrypted.iter().filter(|(_, name, _)| name == n).count(); + assert_eq!(count_named("S1"), 1); + assert_eq!(count_named("S2"), 1); +} + +fn minimal_bundle(name: &str) -> ProjectBundle { + ProjectBundle { + kind: ProjectBundle::KIND.to_string(), + version: ProjectBundle::VERSION, + name: name.to_string(), + description: None, + template: "generic".to_string(), + environments: vec![EnvironmentBundle { + name: "default".to_string(), + is_default: true, + vars: vec![ProjectBundleVar { key: "K".to_string(), item_name: "itemA".to_string() }], + }], + items: vec![PlainItem { + item_type: "secret".to_string(), + name: "itemA".to_string(), + value: Some("v".to_string()), + username: None, + password: None, + url: None, + notes: None, + category: None, + command: None, + }], + } +} + +#[tokio::test] +async fn receive_refuses_case_insensitive_project_name_collision() { + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + + // Receiving vault already has a project named "myapp" (lowercase). + let existing_id = db.upsert_project(0, "myapp", None, "generic").await.unwrap(); + db.upsert_environment(0, existing_id, "default", true).await.unwrap(); + + let before_items = db.list_items().await.unwrap().len(); + let before_envs = db.list_environments(existing_id).await.unwrap().len(); + let before_owned = db.list_owned_item_ids(existing_id).await.unwrap().len(); + let before_projects = db.list_projects().await.unwrap().len(); + + // Incoming bundle uses "MyApp" -- different case, same name. + let bundle = minimal_bundle("MyApp"); + let err = project_relay::receive_project_bundle(&db, &RECEIVER_KEY, bundle, None) + .await + .expect_err("case-insensitive name collision must be a hard error"); + + assert!(err.starts_with("conflict:"), "expected a 'conflict:'-prefixed error, got: {err}"); + + // Vault must be entirely unchanged -- transaction rolled back. + assert_eq!(db.list_items().await.unwrap().len(), before_items, "no new items rows"); + assert_eq!(db.list_environments(existing_id).await.unwrap().len(), before_envs, "no new environments rows"); + assert_eq!(db.list_owned_item_ids(existing_id).await.unwrap().len(), before_owned, "no new item_projects rows"); + assert_eq!(db.list_projects().await.unwrap().len(), before_projects, "no new project row"); +} + +#[tokio::test] +async fn receive_with_name_override_succeeds_after_collision() { + let sender_dir = tempdir().unwrap(); + let (sender_db, project_id, envs) = seed_three_env_project(&sender_dir).await; + let bundle = project_relay::build_project_bundle(&sender_db, &SENDER_KEY, project_id, &envs) + .await + .unwrap(); + assert_eq!(bundle.name, "MyApp"); + + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + // A colliding project already exists in the receiving vault. + db.upsert_project(0, "MyApp", None, "generic").await.unwrap(); + + let result = project_relay::receive_project_bundle( + &db, + &RECEIVER_KEY, + bundle, + Some("MyApp-received".to_string()), + ) + .await + .expect("receive with an override name must succeed despite the collision"); + + assert_eq!(result.project_name, "MyApp-received"); + assert_eq!(result.item_count, 5); + assert_eq!(result.environment_names.len(), 3); + + let projects = db.list_projects().await.unwrap(); + assert_eq!(projects.len(), 2, "original + newly received project"); + assert!(projects.iter().any(|p| p.name == "MyApp-received")); +} + +#[tokio::test] +async fn send_excludes_unselected_environments() { + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + let project_id = db.upsert_project(0, "MyApp", None, "generic").await.unwrap(); + let local = db.upsert_environment(0, project_id, "local", true).await.unwrap(); + let production = db.upsert_environment(0, project_id, "production", false).await.unwrap(); + + let shared = seed_item(&db, &SENDER_KEY, "SHARED", "shared-value").await; + let prod_only = seed_item(&db, &SENDER_KEY, "PRODONLY", "prod-value").await; + + link_var(&db, local, project_id, shared, "SHARED").await; + link_var(&db, production, project_id, shared, "SHARED").await; + link_var(&db, production, project_id, prod_only, "PRODONLY").await; + + // Select only the default (local) environment. + let bundle = project_relay::build_project_bundle(&db, &SENDER_KEY, project_id, &[local]) + .await + .unwrap(); + + assert_eq!(bundle.environments.len(), 1); + assert_eq!(bundle.environments[0].name, "local"); + assert!( + bundle.items.iter().all(|i| i.name != "PRODONLY"), + "an item used only by the unselected 'production' environment must be absent from bundle.items" + ); + assert!(bundle.items.iter().any(|i| i.name == "SHARED")); +} + +#[tokio::test] +async fn send_skips_dangling_item_reference() { + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + let project_id = db.upsert_project(0, "MyApp", None, "generic").await.unwrap(); + let local = db.upsert_environment(0, project_id, "local", true).await.unwrap(); + + let real_item = seed_item(&db, &SENDER_KEY, "REAL", "value").await; + link_var(&db, local, project_id, real_item, "REAL").await; + + // Link a var whose item_id does not (and never will) resolve to a real + // item -- simulates an item deleted after linking. + let dangling_item_id = 999_999; + db.upsert_environment_var(local, "GHOST", dangling_item_id).await.unwrap(); + + let bundle = project_relay::build_project_bundle(&db, &SENDER_KEY, project_id, &[local]) + .await + .expect("a dangling item_id must not fail the whole send"); + + let total_vars: usize = bundle.environments.iter().map(|e| e.vars.len()).sum(); + assert_eq!(total_vars, 1, "the dangling var is skipped, only the real one remains"); + assert_eq!(bundle.items.len(), 1); + assert_eq!(bundle.items[0].name, "REAL"); +} + +#[tokio::test] +async fn received_items_are_project_owned_not_global() { + let sender_dir = tempdir().unwrap(); + let (sender_db, project_id, envs) = seed_three_env_project(&sender_dir).await; + let bundle = project_relay::build_project_bundle(&sender_db, &SENDER_KEY, project_id, &envs) + .await + .unwrap(); + + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + let result = project_relay::receive_project_bundle(&db, &RECEIVER_KEY, bundle, None) + .await + .unwrap(); + + let owned = db.list_owned_item_ids(result.project_id).await.unwrap(); + assert_eq!(owned.len(), 5); + for item_id in owned { + assert_eq!(db.is_item_global(item_id).await.unwrap(), Some(false), "received items must not be global"); + assert_eq!(db.item_owner_count(item_id).await.unwrap(), 1, "exactly one item_projects row per item"); + } +} + +#[tokio::test] +async fn bundle_never_contains_paths() { + let dir = tempdir().unwrap(); + let db = open_db(&dir).await; + let project_id = db.upsert_project(0, "MyApp", None, "generic").await.unwrap(); + let local = db.upsert_environment(0, project_id, "local", true).await.unwrap(); + + let leaky_path = "C:\\Users\\maosuarez\\dev\\myapp\\.env.production"; + db.set_environment_paths(local, &[leaky_path.to_string()]).await.unwrap(); + + let item = seed_item(&db, &SENDER_KEY, "SECRET", "value").await; + link_var(&db, local, project_id, item, "SECRET").await; + + let bundle = project_relay::build_project_bundle(&db, &SENDER_KEY, project_id, &[local]) + .await + .unwrap(); + + let json = serde_json::to_string(&bundle).unwrap(); + assert!(!json.contains("maosuarez"), "bundle JSON must not contain any path substring"); + assert!(!json.contains(".env.production")); + assert!(!json.contains("C:\\Users")); +} diff --git a/src-tauri/tests/vault_integration.rs b/src-tauri/tests/vault_integration.rs index 714f5c6..f151a9c 100644 --- a/src-tauri/tests/vault_integration.rs +++ b/src-tauri/tests/vault_integration.rs @@ -1,7 +1,7 @@ // Integration tests for VaultDb using a real SQLite database in a temp directory. // Run with: cargo test --test vault_integration -use crypt_env_lib::db::VaultDb; +use crypt_env_lib::db::{LinkMode, VaultDb}; use crypt_env_lib::project::{self, EnvironmentInput, EnvironmentVar, ProjectInput}; use crypt_env_lib::vault::{self, VaultItem}; use tempfile::tempdir; @@ -210,3 +210,168 @@ async fn test_db_init_vault_and_get_meta() { assert_eq!(salt, "deadbeef_salt"); assert_eq!(token, "deadbeef_token"); } + +// ─── Issue #9: db-layer tests (12-15 of the plan's test matrix) ─────────── +// +// Tests 1-11 need an HTTP-level harness (bound router + token + unlocked +// temp vault) that issue #11 owns and had not landed in this worktree at the +// time of writing — see the plan's §3.9 and this repo's parallel-worktree +// setup. Consuming that harness for the `api`-level create-or-update-on- +// collision assertions (200/201/409 status codes, `SHARED_ITEM_CONFLICT` / +// `KEY_EXISTS` / `CONFLICT_RETRY` bodies) is a documented follow-up once +// `crate::test_support` / `TestVault` land here. These four are pure +// `VaultDb` tests and need no HTTP surface at all. + +#[tokio::test] +async fn test_link_item_rolls_back_when_link_fails() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + let project_id = db.upsert_project(0, "acme", None, "generic").await.unwrap(); + let bogus_environment_id = 999_999; // no such environment exists + + let result = db + .create_or_link_item( + bogus_environment_id, + project_id, + "API_KEY", + "secret", + "ciphertext-blob", + "2026-01-01T00:00:00Z", + LinkMode::Update, + None, // caller's inspect_env_key saw a free key + ) + .await; + + assert!( + result.is_err(), + "linking into a nonexistent environment must fail (FK violation on environment_vars)" + ); + + let items = db.list_items().await.unwrap(); + assert!(items.is_empty(), "the item insert must be rolled back with the rest of the transaction"); + + let owned = db.list_owned_item_ids(project_id).await.unwrap(); + assert!(owned.is_empty(), "the item_projects insert must be rolled back too — no orphaned ownership row"); +} + +#[tokio::test] +async fn test_list_orphan_items_finds_unlinked_nonglobal() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + let id = db.upsert_item(0, "secret", "ciphertext", "2026-01-01", false).await.unwrap(); + // No environment_vars row created — this item is unreferenced. + + let orphans = db.list_orphan_item_ids().await.unwrap(); + assert_eq!(orphans, vec![id], "an unlinked, non-global item must be reported as an orphan"); +} + +#[tokio::test] +async fn test_list_orphan_items_excludes_global_and_linked() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + // Unlinked but global: has a reachable surface (Global Secrets) even + // without a link, so it must NOT be reported as an orphan. + let global_id = db.upsert_item(0, "secret", "global-ciphertext", "2026-01-01", true).await.unwrap(); + + // Non-global but linked: reachable via its environment's scoped list, + // so it must NOT be reported as an orphan either. + let linked_id = db.upsert_item(0, "secret", "linked-ciphertext", "2026-01-01", false).await.unwrap(); + let project_id = db.upsert_project(0, "acme", None, "generic").await.unwrap(); + let environment_id = db.upsert_environment(0, project_id, "production", true).await.unwrap(); + db.upsert_environment_var(environment_id, "DB_PASSWORD", linked_id).await.unwrap(); + + let orphans = db.list_orphan_item_ids().await.unwrap(); + assert!(!orphans.contains(&global_id), "a global unlinked item must not be reported as an orphan"); + assert!(!orphans.contains(&linked_id), "a linked non-global item must not be reported as an orphan"); + assert!(orphans.is_empty(), "no other rows were created in this test"); +} + +#[tokio::test] +async fn test_prune_orphan_items_clears_item_projects() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + let project_id = db.upsert_project(0, "acme", None, "generic").await.unwrap(); + + // The orphan to be pruned: owned, but never linked into any environment. + let orphan_id = db.upsert_item(0, "secret", "orphan-ciphertext", "2026-01-01", false).await.unwrap(); + db.add_item_owner(orphan_id, project_id).await.unwrap(); + + // A second, unrelated item that must survive the prune untouched. + let kept_id = db.upsert_item(0, "secret", "kept-ciphertext", "2026-01-01", false).await.unwrap(); + db.add_item_owner(kept_id, project_id).await.unwrap(); + let environment_id = db.upsert_environment(0, project_id, "production", true).await.unwrap(); + db.upsert_environment_var(environment_id, "DB_PASSWORD", kept_id).await.unwrap(); + + vault::prune_orphan_items(&db, &[orphan_id]).await.unwrap(); + + let items = db.list_items().await.unwrap(); + assert!( + items.iter().all(|(id, ..)| *id != orphan_id), + "the orphan's items row must be deleted" + ); + assert!( + db.list_owned_item_ids(project_id).await.unwrap().iter().all(|id| *id != orphan_id), + "the orphan's item_projects row must be deleted too" + ); + + assert!(items.iter().any(|(id, ..)| *id == kept_id), "an item not in the prune list must survive"); + assert!( + db.list_owned_item_ids(project_id).await.unwrap().contains(&kept_id), + "the surviving item's ownership link must be untouched" + ); +} + +// ─── Issue #13: global-item scoped visibility — data-path guards ───────────── +// +// The API's scope filter (src-tauri/src/api/mod.rs, `scope_items`) now reads +// `is_global` off every row returned by `list_items()` to decide whether an +// unlinked item should be unioned into a discovery response. These two tests +// guard the data path that decision depends on: the DB layer must round-trip +// `is_global` faithfully through both insert/update (`upsert_item`) and the +// dedicated toggle (`set_item_global`). + +#[tokio::test] +async fn test_db_list_items_preserves_is_global_flag() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + let global_id = db.upsert_item(0, "secret", "global_data", "2026-01-01", true).await.unwrap(); + let non_global_id = db.upsert_item(0, "secret", "scoped_data", "2026-01-01", false).await.unwrap(); + + let items = db.list_items().await.unwrap(); + assert_eq!(items.len(), 2); + + let global_row = items.iter().find(|r| r.0 == global_id).expect("global item must be present"); + assert!(global_row.4, "tuple index 4 (is_global) must be true for the item created global"); + + let scoped_row = items.iter().find(|r| r.0 == non_global_id).expect("non-global item must be present"); + assert!(!scoped_row.4, "tuple index 4 (is_global) must be false for the item created non-global"); +} + +#[tokio::test] +async fn test_db_set_item_global_roundtrip() { + let dir = tempdir().unwrap(); + let db_path = dir.path().join("test.db").to_str().unwrap().to_string(); + let db = VaultDb::open(&db_path).await.unwrap(); + + let id = db.upsert_item(0, "secret", "data", "2026-01-01", false).await.unwrap(); + + db.set_item_global(id, true).await.unwrap(); + let items = db.list_items().await.unwrap(); + let row = items.iter().find(|r| r.0 == id).unwrap(); + assert!(row.4, "set_item_global(true) must flip is_global to true in list_items"); + + db.set_item_global(id, false).await.unwrap(); + let items = db.list_items().await.unwrap(); + let row = items.iter().find(|r| r.0 == id).unwrap(); + assert!(!row.4, "set_item_global(false) must flip is_global back to false in list_items"); +} diff --git a/src/components/ProjectManager.tsx b/src/components/ProjectManager.tsx index 036c271..12d044c 100644 --- a/src/components/ProjectManager.tsx +++ b/src/components/ProjectManager.tsx @@ -3,6 +3,7 @@ import { invoke } from '@tauri-apps/api/core'; import { platform } from '@tauri-apps/plugin-os'; import { Icon } from './ui/Icon'; import { TagInput } from './ui/TagInput'; +import { ProjectShareModal, type ProjectShareMode } from './ProjectShareModal'; import { useVaultStore } from '../store'; import { useProjectStore } from '../store/projectStore'; import { @@ -40,6 +41,33 @@ const TEMPLATES: { id: ProjectTemplate; label: string; vars: string[] }[] = [ const ENV_PRESETS = ['production', 'local', 'test', 'staging']; +// ─── Name validation (UX mirror only — see issue #7) ────────────────────────── +// +// These mirror `validate_environment_name` / `validate_project_name` in +// `src-tauri/src/project/mod.rs` purely to avoid a pointless round-trip to +// the server. The server is the actual enforcement point (reachable from +// the GUI, HTTP API, CLI, and an imported `.cryptenv-proj` template) — this +// copy may drift from it over time and that is an accepted risk, not a bug. + +const ENV_NAME_RULE = + "must be 1-64 chars, start with a letter or digit, and contain only letters, digits, '.', '_' or '-'"; + +function isValidEnvironmentName(name: string): boolean { + return /^[A-Za-z0-9][A-Za-z0-9._-]{0,63}$/.test(name) && !name.endsWith('.') && !name.endsWith('-'); +} + +const PROJECT_NAME_RULE = + 'must not contain \'/\', \'\\\', control characters, \':\', \'<\', \'>\', \'"\', \'|\', \'?\', \'*\', or start/end with \'.\' or whitespace (128 chars max)'; + +function isValidProjectName(name: string): boolean { + if (name.trim().length === 0 || name.length > 128) return false; + if (name === '.' || name === '..') return false; + // eslint-disable-next-line no-control-regex + if (/[\x00-\x1f/\\:<>"|?*]/.test(name)) return false; + if (name.endsWith('.') || name.endsWith(' ') || name.startsWith('.') || name.startsWith(' ')) return false; + return true; +} + function TemplateModal({ onSelect, onClose, @@ -628,6 +656,7 @@ export function ProjectManager() { const [isCreatingProj, setIsCreatingProj] = useState(false); const [templateModal, setTemplateModal] = useState(false); const [confirmDelProj, setConfirmDelProj] = useState(false); + const [shareModalMode, setShareModalMode] = useState(null); // Environment form state const [envName, setEnvName] = useState(''); @@ -754,7 +783,7 @@ export function ProjectManager() { }; const handleSaveProject = async () => { - if (!projName.trim()) { showToast('Name is required', 'error'); return; } + if (!isValidProjectName(projName.trim())) { showToast(`Name ${PROJECT_NAME_RULE}`, 'error'); return; } setSaving(true); try { const id = await saveProject({ @@ -907,7 +936,7 @@ export function ProjectManager() { const handleSaveEnvironment = async () => { if (!selectedProject) return; - if (!envName.trim()) { showToast('Name is required', 'error'); return; } + if (!isValidEnvironmentName(envName.trim())) { showToast(`Name ${ENV_NAME_RULE}`, 'error'); return; } setSaving(true); try { await saveEnvironment({ @@ -973,6 +1002,12 @@ export function ProjectManager() { > LOAD TEMPLATE + + ); +} + +function BtnSecondary({ children, onClick, disabled }: { children: React.ReactNode; onClick?: () => void; disabled?: boolean }) { + return ( + + ); +} + +function InlineError({ msg }: { msg: string }) { + return ( +
+ {msg} +
+ ); +} + +export function ProjectShareModal({ mode, project, items, onClose, onReceived }: ProjectShareModalProps) { + const itemsById = useMemo(() => new Map(items.map((i) => [i.id, i])), [items]); + + // ── Send state ── + const [selectedEnvIds, setSelectedEnvIds] = useState>(() => { + if (!project || project.environments.length === 0) return new Set(); + const def = project.environments.find((e) => e.isDefault); + return new Set([def ? def.id : project.environments[0].id]); + }); + const [sending, setSending] = useState(false); + const [sendResult, setSendResult] = useState(null); + + // ── Receive state ── + const [code, setCode] = useState(''); + const [passphrase, setPassphrase] = useState(''); + const [overrideName, setOverrideName] = useState(''); + const [showRename, setShowRename] = useState(false); + const [receiving, setReceiving] = useState(false); + const [receiveResult, setReceiveResult] = useState(null); + + const [error, setError] = useState(''); + + const toggleEnv = (id: number) => { + setSelectedEnvIds((prev) => { + const next = new Set(prev); + if (next.has(id)) next.delete(id); else next.add(id); + return next; + }); + }; + + const handleSend = async () => { + if (!project || selectedEnvIds.size === 0) return; + setSending(true); + setError(''); + try { + const result = await invoke('project_relay_send', { + projectId: project.id, + environmentIds: Array.from(selectedEnvIds), + }); + setSendResult(result); + } catch (e) { + setError(String(e)); + } finally { + setSending(false); + } + }; + + const handleReceive = async () => { + if (!code || !passphrase) return; + setReceiving(true); + setError(''); + try { + const result = await invoke('project_relay_receive', { + code: code.toUpperCase(), + passphrase, + projectNameOverride: overrideName.trim() || null, + }); + setReceiveResult(result); + onReceived?.(); + } catch (e) { + const msg = String(e); + if (msg.includes('conflict:')) { + setShowRename(true); + const match = msg.match(/'([^']+)'/); + if (match && !overrideName) setOverrideName(`${match[1]}-received`); + setError('A project with this name already exists here. Choose a different name below and try again.'); + } else { + setError(msg); + } + } finally { + setReceiving(false); + } + }; + + function renderSend() { + if (!project) return null; + + if (sendResult) { + return ( + <> +
+
+ +
+
Uploaded successfully
+
+
+ {sendResult.environmentCount} environment{sendResult.environmentCount !== 1 ? 's' : ''}, {sendResult.itemCount} item{sendResult.itemCount !== 1 ? 's' : ''} +
+ +
+ DONE +
+ + ); + } + + if (project.environments.length === 0) { + return ( + <> + +
+ CLOSE +
+ + ); + } + + return ( + <> +
+ Select which environments of {project.name} to share. + Non-default environments start unchecked — over-sharing is unrecoverable, under-sharing just costs one more send. +
+ +
+ {project.environments.map((env) => { + const checked = selectedEnvIds.has(env.id); + const prodLike = looksProduction(env.name); + return ( + + ); + })} +
+ + {selectedEnvIds.size > 0 && ( +
+
KEYS THAT WILL LEAVE THIS MACHINE (never values)
+
+ {project.environments + .filter((e) => selectedEnvIds.has(e.id)) + .map((env) => ( +
+
{env.name.toUpperCase()}
+ {env.vars.length === 0 ? ( +
no variables
+ ) : ( + env.vars.map((v) => ( +
+ {v.key} + {'->'} + {displayName(itemsById.get(v.itemId), v.itemId)} +
+ )) + )} +
+ ))} +
+
+ )} + + {error && } + +
+ CANCEL + + {sending ? 'UPLOADING…' : 'SHARE PROJECT'} + +
+ + ); + } + + function renderReceive() { + if (receiveResult) { + return ( + <> +
+
+ +
+
Project received
+
+
+ {receiveResult.project} — {receiveResult.itemCount} item{receiveResult.itemCount !== 1 ? 's' : ''} +
+
+ {receiveResult.environments.map((name) => ( +
+ + {name} +
+ ))} +
+
+ Received items are owned by this project only (not global). Set paths on each environment before injecting. +
+
+ DONE +
+ + ); + } + + return ( + <> +
+ Enter the code and passphrase from the sender. This always creates a new project — it never merges into an existing one. +
+
+ + setCode(e.target.value.toUpperCase())} + placeholder="XXXX-XXXX" + maxLength={9} + className="w-full h-9 bg-raised border border-bd2 rounded-[3px] px-3 text-[15px] font-mono text-accent tracking-[0.2em] placeholder:text-tx3 outline-none focus:border-accent transition-colors" + /> +
+
+ + setPassphrase(e.target.value)} + onKeyDown={(e) => { if (e.key === 'Enter' && !showRename) handleReceive(); }} + className="w-full h-9 bg-raised border border-bd2 rounded-[3px] px-3 text-[13px] font-mono text-tx placeholder:text-tx3 outline-none focus:border-accent transition-colors" + /> +
+ + {showRename && ( +
+ + setOverrideName(e.target.value)} + onKeyDown={(e) => { if (e.key === 'Enter') handleReceive(); }} + className="w-full h-9 bg-raised border border-bd2 rounded-[3px] px-3 text-[13px] font-mono text-tx placeholder:text-tx3 outline-none focus:border-accent transition-colors" + /> +
+ )} + + {error && } + +
+ CANCEL + + {receiving ? 'DOWNLOADING…' : 'RECEIVE'} + +
+ + ); + } + + return ( +
+
+
+
+ + + {mode === 'send' ? 'SHARE PROJECT' : 'RECEIVE PROJECT'} + +
+ +
+ + {mode === 'send' ? renderSend() : renderReceive()} +
+
+ ); +} diff --git a/src/components/ShareModal.tsx b/src/components/ShareModal.tsx index 6d8ec89..04455b1 100644 --- a/src/components/ShareModal.tsx +++ b/src/components/ShareModal.tsx @@ -2,6 +2,7 @@ import { useState, useEffect, useRef, useCallback } from 'react'; import { invoke } from '@tauri-apps/api/core'; import { writeText } from '@tauri-apps/plugin-clipboard-manager'; import { Icon } from './ui/Icon'; +import { RelayCodeDisplay } from './ui/RelayCodeDisplay'; // --------------------------------------------------------------------------- // Types @@ -338,8 +339,6 @@ export function ShareModal({ selectedIds, onClose, onImportDone, onSendDone }: S const [relayRxPass, setRelayRxPass] = useState(''); const [relayRxLoading, setRelayRxLoading] = useState(false); const [relayRxNames, setRelayRxNames] = useState([]); - const [copiedRelayCode, setCopiedRelayCode] = useState(false); - const [copiedRelayPass, setCopiedRelayPass] = useState(false); // Error state const [error, setError] = useState(''); @@ -1051,8 +1050,6 @@ export function ShareModal({ selectedIds, onClose, onImportDone, onSendDone }: S } function renderInternetDoneSend() { - const copyCode = () => { navigator.clipboard.writeText(relayCode); setCopiedRelayCode(true); setTimeout(() => setCopiedRelayCode(false), 2000); }; - const copyPass = () => { navigator.clipboard.writeText(relayPassphrase); setCopiedRelayPass(true); setTimeout(() => setCopiedRelayPass(false), 2000); }; return ( <> @@ -1065,31 +1062,8 @@ export function ShareModal({ selectedIds, onClose, onImportDone, onSendDone }: S Send BOTH to your teammate via any channel -
-
CODE
-
- {relayCode} - -
-
- -
-
PASSPHRASE
-
- {relayPassphrase} - -
-
+ -

- The relay link expires in 24 hours and is destroyed after first use. Never share code + passphrase in the same message. -

{ onSendDone?.(); onClose(); }}>DONE diff --git a/src/components/ui/RelayCodeDisplay.tsx b/src/components/ui/RelayCodeDisplay.tsx new file mode 100644 index 0000000..cf23f7a --- /dev/null +++ b/src/components/ui/RelayCodeDisplay.tsx @@ -0,0 +1,67 @@ +import { useState } from 'react'; +import { Icon } from './Icon'; + +/** + * Two-box "code" + "passphrase" display with copy buttons, used after a + * successful relay upload. Extracted out of `ShareModal.tsx`'s internet-send + * "done" step so `ProjectShareModal.tsx` (whole-project relay share, issue + * #4) doesn't duplicate the same markup — behavior is unchanged from the + * original inline version. + */ +export function RelayCodeDisplay({ code, passphrase }: { code: string; passphrase: string }) { + const [copiedCode, setCopiedCode] = useState(false); + const [copiedPass, setCopiedPass] = useState(false); + + const copyCode = () => { + navigator.clipboard.writeText(code); + setCopiedCode(true); + setTimeout(() => setCopiedCode(false), 2000); + }; + const copyPass = () => { + navigator.clipboard.writeText(passphrase); + setCopiedPass(true); + setTimeout(() => setCopiedPass(false), 2000); + }; + + return ( + <> +
+
CODE
+
+ {code} + +
+
+ +
+
PASSPHRASE
+
+ {passphrase} + +
+
+ +

+ The relay link expires in 24 hours and is destroyed after first use. Never share code + passphrase in the same message. +

+ + ); +}