docs(spec): v0.5.0 polish + harden bundle
Anchors on a HIGH-severity auth bypass in the relicario-server pre-receive hook (revocation + registered-device checks both unimplemented), bundles two hardening follow-ups, two confirmed bugs, and four UX improvements. Splits into Plan A (Rust + docs) and Plan B (extension UX) for independent merge cadence. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
adlee-was-taken
340
docs/superpowers/specs/2026-05-02-v0.5.0-polish-harden-design.md
Normal file
340
docs/superpowers/specs/2026-05-02-v0.5.0-polish-harden-design.md
Normal file
@@ -0,0 +1,340 @@
|
|||||||
|
# v0.5.0 — Polish + Harden — Design
|
||||||
|
|
||||||
|
Date: 2026-05-02
|
||||||
|
Status: Draft
|
||||||
|
|
||||||
|
## Overview
|
||||||
|
|
||||||
|
v0.5.0 is a "polish + harden" bundle: a security-vulnerability fix, two
|
||||||
|
hardening follow-ups, two confirmed bugs, and four UX improvements.
|
||||||
|
No new functional features. Functional work — LastPass import, recovery
|
||||||
|
QR, Plan 1C-γ, Fullscreen Phases 3/4 — proceeds on parallel plans.
|
||||||
|
|
||||||
|
The anchor item is a **HIGH-severity authentication bypass** in
|
||||||
|
`relicario-server`'s pre-receive hook (S1): both the registered-device
|
||||||
|
check and the revocation check are unimplemented. Until this lands, the
|
||||||
|
device-auth system is a no-op.
|
||||||
|
|
||||||
|
## Goals
|
||||||
|
|
||||||
|
1. Restore device-auth integrity: only registered, non-revoked signing
|
||||||
|
keys can push to the vault repo.
|
||||||
|
2. Close two minor hardening gaps surfaced during the security review.
|
||||||
|
3. Fix two user-visible bugs (strength-meter desync after generate,
|
||||||
|
raw error codes in the fullscreen tab).
|
||||||
|
4. Deliver four UX improvements that are polish-shaped, not feature-shaped.
|
||||||
|
5. Tag v0.5.0 with no known security vulnerabilities or visible-error-code
|
||||||
|
leaks.
|
||||||
|
|
||||||
|
## Scope Map
|
||||||
|
|
||||||
|
| Bucket | Item | Plan |
|
||||||
|
|---|---|---|
|
||||||
|
| Security | S1: Pre-receive hook fix | A |
|
||||||
|
| Security | S2: Tar archive path-traversal hardening | A |
|
||||||
|
| Security | S3: `RELICARIO_*` env-var audit | A |
|
||||||
|
| Cleanup | C1: Stale feature branch prune | A |
|
||||||
|
| Bugs | B1: Strength meter desync after regenerate | B |
|
||||||
|
| Bugs | B2: `vault_locked` raw error code in fullscreen tab | B (subsumed by P4) |
|
||||||
|
| UX | P1: Password coloring | B |
|
||||||
|
| UX | P2: Setup-wizard completion → fullscreen vault tab | B |
|
||||||
|
| UX | P3: Form-layout 2-col → full-width transition | B |
|
||||||
|
| UX | P4: Error-message audit (snake_case codes → friendly copy) | B |
|
||||||
|
|
||||||
|
Two plans split by language/blast-radius:
|
||||||
|
- **Plan A** = Rust + docs (server, CLI, env-var audit, branch cleanup)
|
||||||
|
- **Plan B** = Extension UX (TypeScript + CSS)
|
||||||
|
|
||||||
|
The plans share no files and can ship independently.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Plan A — Security + Cleanup
|
||||||
|
|
||||||
|
### S1. Pre-Receive Hook Fix (anchor)
|
||||||
|
|
||||||
|
**Problem.** `crates/relicario-server/src/main.rs:36-81`: `verify_commit`
|
||||||
|
loads `devices` and `revoked` from the repo at the commit, then drops
|
||||||
|
both on the floor. It runs `git verify-commit --raw` and accepts on
|
||||||
|
`GOODSIG` / `Good signature` regardless of which key produced the
|
||||||
|
signature. This means:
|
||||||
|
|
||||||
|
- An attacker who never registered a device can push commits signed
|
||||||
|
with any GPG key that the server's keyring/allowed-signers happens
|
||||||
|
to trust (or none at all if the server has no SSH allowed-signers
|
||||||
|
configured).
|
||||||
|
- A revoked device's signing key continues to authenticate after
|
||||||
|
`relicario device revoke` is run — the `revoked.json` write is
|
||||||
|
cosmetic.
|
||||||
|
|
||||||
|
**Fix.** Implement the verification logic per the design spec
|
||||||
|
(`docs/superpowers/specs/2026-05-02-device-authentication-design.md:284-300`):
|
||||||
|
|
||||||
|
1. Build a temporary allowed-signers file from `devices.json` entries
|
||||||
|
loaded at the commit. Pass it to `git verify-commit` via
|
||||||
|
`GIT_CONFIG_COUNT=1 GIT_CONFIG_KEY_0=gpg.ssh.allowedSignersFile
|
||||||
|
GIT_CONFIG_VALUE_0=<tmpfile>` so we don't mutate global git config.
|
||||||
|
2. Parse the signing-key fingerprint out of `git verify-commit --raw`
|
||||||
|
output (the fingerprint appears on the `Good "git" signature for …
|
||||||
|
with … key SHA256:…` line for SSH).
|
||||||
|
3. Reject if the fingerprint is not in `devices.json` at the commit.
|
||||||
|
4. Reject if the fingerprint is in `revoked.json` AND
|
||||||
|
`commit_timestamp >= revoked_at`. The historical-commit case
|
||||||
|
(timestamp < `revoked_at`) is allowed so old commits survive
|
||||||
|
revocation.
|
||||||
|
5. Use the commit's **committer date** (`GIT_COMMITTER_DATE`,
|
||||||
|
accessible via `git show -s --format=%ct`) — *not* the current wall
|
||||||
|
clock and not the author date — as `commit_timestamp`. Committer
|
||||||
|
date is when the signature was applied; author date is when work
|
||||||
|
was originally written and could be from before revocation even
|
||||||
|
for a malicious replay.
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- An integration test that registers a device, revokes it, signs a
|
||||||
|
commit with the revoked key dated AFTER `revoked_at`, and confirms
|
||||||
|
the hook exits non-zero.
|
||||||
|
- An integration test that signs a commit with a key that was never
|
||||||
|
registered and confirms the hook exits non-zero.
|
||||||
|
- An integration test that signs a commit with a registered, non-revoked
|
||||||
|
key and confirms the hook exits zero.
|
||||||
|
- An integration test that signs a commit with a revoked key dated
|
||||||
|
BEFORE `revoked_at` (historical case) and confirms the hook exits zero.
|
||||||
|
- The dead `let _ = &revoked;` line is gone.
|
||||||
|
|
||||||
|
### S2. Tar Archive Path-Traversal Hardening
|
||||||
|
|
||||||
|
**Problem.** `crates/relicario-cli/src/main.rs:1722` unpacks the
|
||||||
|
bundled `git_archive` from a `.relbak` via `tar::Archive::unpack`,
|
||||||
|
trusting the `tar` crate's defaults. A malicious `.relbak` could
|
||||||
|
contain entries with `..` components or absolute paths.
|
||||||
|
|
||||||
|
**Fix.** Iterate entries explicitly:
|
||||||
|
|
||||||
|
- Reject any entry whose path contains a `..` component, an absolute
|
||||||
|
prefix, or a Windows drive letter.
|
||||||
|
- Reject symlinks and hardlinks (we never write either inside `.git/`
|
||||||
|
on restore).
|
||||||
|
- Cap total uncompressed size at 100× the compressed `git_archive`
|
||||||
|
size or 1 GiB, whichever is lower (defends against tar bombs).
|
||||||
|
- Write each entry under `target.join(".git")` only after the path
|
||||||
|
resolves *inside* that directory.
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- Unit test with a hand-crafted tar containing `../../etc/passwd` —
|
||||||
|
restore bails with "path traversal blocked".
|
||||||
|
- Unit test with a symlink entry — restore bails.
|
||||||
|
- Unit test with a 1 TiB sparse entry — restore bails on the size cap.
|
||||||
|
- Existing valid-restore test still passes.
|
||||||
|
|
||||||
|
### S3. `RELICARIO_*` Env-Var Audit
|
||||||
|
|
||||||
|
**Problem.** Three env vars are not gated by `cfg(debug_assertions)`:
|
||||||
|
- `RELICARIO_IMAGE` (`crates/relicario-cli/src/session.rs:125`) — reference-image override path.
|
||||||
|
- `RELICARIO_NO_GROUPS_CACHE` (`crates/relicario-cli/src/helpers.rs:103`) — disables the groups cache.
|
||||||
|
- `RELICARIO_GITEA_URL` (`crates/relicario-cli/src/main.rs:2298`) — Gitea base URL fallback.
|
||||||
|
|
||||||
|
Spot-check confirmed these look intentional, but they're undocumented
|
||||||
|
and `RELICARIO_NO_GROUPS_CACHE` is more of a dev escape hatch than
|
||||||
|
production config.
|
||||||
|
|
||||||
|
**Fix.**
|
||||||
|
- Document each env var in `docs/SECURITY.md` and the relevant
|
||||||
|
`--help` text.
|
||||||
|
- Move `RELICARIO_NO_GROUPS_CACHE` under `cfg(debug_assertions)` —
|
||||||
|
it's a dev tool, not a user knob.
|
||||||
|
- Leave `RELICARIO_IMAGE` and `RELICARIO_GITEA_URL` as user-facing
|
||||||
|
config; audit each call site to confirm no untrusted-input-flows-into-path
|
||||||
|
cases.
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- `docs/SECURITY.md` lists every `RELICARIO_*` env var with purpose
|
||||||
|
and trust assumption.
|
||||||
|
- `RELICARIO_NO_GROUPS_CACHE` is no-op in `cargo build --release`.
|
||||||
|
|
||||||
|
### C1. Stale Feature Branch Prune
|
||||||
|
|
||||||
|
**Problem.** Six local branches with no merged history beyond what's
|
||||||
|
already in main: `feature/fullscreen-ux-phase-2a`,
|
||||||
|
`feature/typed-items-1a-rust-core`, `feature/typed-items-1c-alpha`,
|
||||||
|
`feature/typed-items-1c-beta1`, `feature/typed-items-1c-beta2`.
|
||||||
|
|
||||||
|
**Fix.** Verify each is fully merged into main (`git branch --merged
|
||||||
|
main`), then delete locally only — no remote branch operations. Plan
|
||||||
|
execution will surface the list and prompt the user before running
|
||||||
|
`git branch -D` per branch (per project rule on git-destructive ops).
|
||||||
|
|
||||||
|
**Acceptance.** `git branch` shows only `main` and active feature
|
||||||
|
branches.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Plan B — Extension UX
|
||||||
|
|
||||||
|
### B1. Strength Meter Desync After Regenerate
|
||||||
|
|
||||||
|
**Problem.** Confirmed in screenshot from 2026-05-02: clicking the
|
||||||
|
regenerate button on a login form's password field rerolls a
|
||||||
|
20-character mixed-class password (e.g., `sCMtTJkF%GN^mF#-N6D%`) but
|
||||||
|
the strength meter still reports `~10^1 guesses — trivially crackable`.
|
||||||
|
The rated string is stale — likely whatever was in the field before
|
||||||
|
the reroll, or empty.
|
||||||
|
|
||||||
|
The meter listens to `input` events on the password field
|
||||||
|
(`extension/src/shared/form-affordances/password-tools.ts:65`). The
|
||||||
|
regenerate handler sets `input.value = newPassword` programmatically,
|
||||||
|
which **does not fire** `input` events in standard DOM behavior.
|
||||||
|
|
||||||
|
**Fix.** In the regenerate handler (search for the orange-spinner
|
||||||
|
button click), after assigning `input.value`, dispatch an
|
||||||
|
`InputEvent('input', { bubbles: true })`. Confirm by re-rating
|
||||||
|
inside the handler if needed.
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- Vitest: simulate regenerate click, assert that `input` event is
|
||||||
|
dispatched and meter calls `scheduleRate` with the new value.
|
||||||
|
- Manual: regenerate a password on the login form, confirm meter
|
||||||
|
jumps to "strong" with appropriate `guesses_log10`.
|
||||||
|
|
||||||
|
### P4. Error-Message Audit (subsumes B2)
|
||||||
|
|
||||||
|
**Problem.** Snake_case error codes leak straight into the fullscreen
|
||||||
|
tab and other surfaces:
|
||||||
|
|
||||||
|
- `vault_locked` shown as a red string in the new-login form
|
||||||
|
(screenshot 2026-05-02).
|
||||||
|
- ~20 call sites in `extension/src/service-worker/router/` return
|
||||||
|
`{ ok: false, error: 'vault_locked' }` and similar.
|
||||||
|
|
||||||
|
The popup has a one-off mapping at `extension/src/popup/popup.ts:147`
|
||||||
|
(`/vault_locked/i.test(err)` → unlock prompt), but the fullscreen tab
|
||||||
|
has no equivalent.
|
||||||
|
|
||||||
|
**Fix.**
|
||||||
|
1. Build a single `ERROR_COPY` map keyed by error code, returning
|
||||||
|
`{ title: string; body: string; cta?: { label: string; action: () => void } }`.
|
||||||
|
Place at `extension/src/shared/error-copy.ts`.
|
||||||
|
2. Audit call sites in `extension/src/service-worker/router/` for all
|
||||||
|
error codes; ensure each has a mapping.
|
||||||
|
3. Replace the regex in `popup.ts:147` and the raw-error rendering in
|
||||||
|
the fullscreen tab with `lookupErrorCopy(code)`.
|
||||||
|
4. For `vault_locked` specifically: the CTA is "Unlock vault" → opens
|
||||||
|
the popup unlock view (or in the fullscreen tab, the inline unlock
|
||||||
|
form).
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- No raw `snake_case` strings render in any UI surface for any error
|
||||||
|
return path. Verified by grep + manual trigger of each error code.
|
||||||
|
- `vault_locked` in the fullscreen tab shows friendly copy + an
|
||||||
|
"Unlock vault" button that works.
|
||||||
|
- Vitest: enumerate every distinct `error: '...'` string literal
|
||||||
|
found in `extension/src/service-worker/router/` via grep, assert
|
||||||
|
each is a key in `ERROR_COPY`. Generated test or build-time check
|
||||||
|
preferred over a static snapshot so it can't drift.
|
||||||
|
|
||||||
|
### P1. Password Coloring
|
||||||
|
|
||||||
|
**Problem / Fix.** Implement per existing spec at
|
||||||
|
`docs/superpowers/specs/2026-05-01-password-coloring-design.md` and plan
|
||||||
|
at `docs/superpowers/plans/2026-05-01-password-coloring.md`. No design
|
||||||
|
changes; pulled into v0.5.0 because it's polish-flavored and small.
|
||||||
|
|
||||||
|
**Acceptance.** Per existing plan's acceptance criteria.
|
||||||
|
|
||||||
|
### P2. Setup-Wizard Completion → Fullscreen Vault Tab
|
||||||
|
|
||||||
|
**Problem.** Setup wizard currently routes to the popup item-list view
|
||||||
|
on completion. The user wants to land in the fullscreen vault tab —
|
||||||
|
the experience reads as more "real software" and avoids the sub-300px
|
||||||
|
popup width on first run.
|
||||||
|
|
||||||
|
**Fix.** In the setup wizard's terminal step, after the vault is
|
||||||
|
created and the device is registered:
|
||||||
|
|
||||||
|
1. Open the fullscreen vault tab via `chrome.tabs.create({ url:
|
||||||
|
chrome.runtime.getURL('vault.html') })`.
|
||||||
|
2. Close the setup tab (or the popup, depending on entry point).
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- Manual: complete the new-vault setup flow; vault tab opens, setup
|
||||||
|
tab closes.
|
||||||
|
- Manual: complete the attach-existing flow; same behavior.
|
||||||
|
- Vitest: assert `chrome.tabs.create` is called with the vault URL on
|
||||||
|
successful setup completion.
|
||||||
|
|
||||||
|
### P3. Form-Layout 2-col → Full-Width Transition
|
||||||
|
|
||||||
|
**Problem.** Screenshot 2026-05-02 shows the new-login form in the
|
||||||
|
fullscreen tab: the IDENTITY and CREDENTIALS cards sit in a 2-column
|
||||||
|
grid with a max-width that stops well short of viewport edge, but the
|
||||||
|
Notes textarea, custom-fields disclosure, and attachments disclosure
|
||||||
|
below all stretch full-width. The visual rhythm breaks at the
|
||||||
|
transition — the lower sections look like a different page.
|
||||||
|
|
||||||
|
**Fix.** Two candidate approaches; pick at implementation time:
|
||||||
|
|
||||||
|
- **A.** Constrain the lower sections to the same max-width and
|
||||||
|
horizontal alignment as the cards. They become a third "row" in the
|
||||||
|
same column system. Simpler, less code.
|
||||||
|
- **B.** Wrap the lower sections in a card matching the upper cards'
|
||||||
|
visual treatment. Notes-as-card, custom-fields-as-card,
|
||||||
|
attachments-as-card. More work, more visual consistency, but might
|
||||||
|
feel heavier.
|
||||||
|
|
||||||
|
Recommended: **A** for v0.5.0 — minimal CSS change, immediate
|
||||||
|
correctness. Revisit B in Phase 3 if the user still feels the layout
|
||||||
|
is off.
|
||||||
|
|
||||||
|
**Acceptance.**
|
||||||
|
- The new-login form (and all item-type forms — login, secure note,
|
||||||
|
identity, card, key, document, totp) renders with consistent
|
||||||
|
horizontal alignment from top of form to bottom.
|
||||||
|
- Manual: viewport at 1920×1080, 1440×900, 1024×768, and 768×1024;
|
||||||
|
no jarring width transitions.
|
||||||
|
|
||||||
|
---
|
||||||
|
|
||||||
|
## Testing Strategy
|
||||||
|
|
||||||
|
| Layer | Tests |
|
||||||
|
|---|---|
|
||||||
|
| Rust unit (`relicario-server`) | S1 acceptance set (4 scenarios) |
|
||||||
|
| Rust unit (`relicario-core`) | S2 path-traversal scenarios (3 scenarios) |
|
||||||
|
| CLI integration | S2 happy-path restore unchanged |
|
||||||
|
| Vitest (extension) | B1 input-event dispatch; P4 ERROR_COPY snapshot; P2 tabs.create call |
|
||||||
|
| Manual | P3 viewport sweep; B2/P4 trigger each error code; S1 hook setup on a real Gitea instance; C1 branch cleanup confirmation |
|
||||||
|
|
||||||
|
Existing tests stay green. No regression budget.
|
||||||
|
|
||||||
|
## Out of Scope
|
||||||
|
|
||||||
|
Listed for clarity; each tracked separately:
|
||||||
|
|
||||||
|
- LastPass import (`docs/superpowers/plans/2026-04-29-relicario-lastpass-import.md`)
|
||||||
|
- Recovery QR + entropy floor (`docs/superpowers/plans/2026-05-01-recovery-qr-and-entropy-floor.md`)
|
||||||
|
- Plan 1C-γ (attachments + Document + trash UI)
|
||||||
|
- Fullscreen Phase 3 (shell) and Phase 4 (palette)
|
||||||
|
- Pre-v0.3.0 manual test walk (`docs/test-checklists/2026-04-27-pre-v0.3.0-audit.md`) — that's a v0.3.0 release gate, not a v0.5.0 item
|
||||||
|
|
||||||
|
## Sequencing
|
||||||
|
|
||||||
|
1. Plan A and Plan B can run in parallel (no shared files).
|
||||||
|
2. Inside Plan A: S1 first (unblocks the security claim), then S2,
|
||||||
|
then S3, then C1.
|
||||||
|
3. Inside Plan B: P4 first (unblocks B2 and surfaces all error codes
|
||||||
|
centrally), then B1, then P1, then P3, then P2 (P2 is end-to-end
|
||||||
|
and benefits from earlier polish).
|
||||||
|
4. Both plans merge to main; tag `v0.5.0` after both PRs are green.
|
||||||
|
|
||||||
|
## Risks
|
||||||
|
|
||||||
|
- **S1 SSH allowed-signers parsing.** `git verify-commit --raw`
|
||||||
|
output format for SSH signatures differs slightly from GPG; the
|
||||||
|
fingerprint-extraction regex needs unit coverage against real
|
||||||
|
output. Mitigation: capture sample outputs from a test repo and
|
||||||
|
check them into the test fixtures.
|
||||||
|
- **P3 layout regressions.** Constraining the lower sections may
|
||||||
|
affect the textarea behavior at small widths. Mitigation: viewport
|
||||||
|
sweep in acceptance.
|
||||||
|
- **C1 accidental deletion.** Plan execution must `git branch
|
||||||
|
--merged main` filter and prompt before each `-D`. The
|
||||||
|
no-destructive-without-asking project rule applies.
|
||||||
Reference in New Issue
Block a user