morphit/docs/PHASE-G-PREP-AUDIT.md

# Phase G prep — security audit of post-F.5 task changes

**Auditor:** Claude (Agorise's collaborator), per task #15.
**Date:** 2026-04-28.
**Scope:** New code introduced by the 14-task post-F.5 campaign.
Pre-existing code is out of scope (covered by `PHASE-F-AUDIT.md`).

This document re-walks each new surface and documents:
- What threat models apply.
- What checks were performed.
- Whether any issues were found.
- What was fixed (if anything) and where.

Severity scale matches `PHASE-F-AUDIT.md`:
- **HIGH** — exploitable today, leads to fund loss, account
  compromise, or privacy violation.
- **MEDIUM** — exploitable but bounded impact, OR requires
  unusual preconditions.
- **LOW** — defense-in-depth gap; theoretical exploit, or
  exploit requires already-compromised position.

## Summary

| ID    | Surface                              | Severity | Status |
|-------|--------------------------------------|----------|--------|
| G-1   | `morphit-ops edit` parser robustness | LOW      | reviewed, no action needed |
| G-2   | `morphit-ops edit` quoting           | LOW      | reviewed, no action needed |
| G-3   | `/v1/chain-fee` cache poisoning      | LOW      | reviewed, no action needed |
| G-4   | `/v1/chain-fee` RPC hostile response | LOW      | reviewed, no action needed |
| G-5   | SEO override storage XSS             | MEDIUM   | reviewed, no action needed |
| G-6   | Alt-network keystore passphrase reuse| LOW      | accepted tradeoff, documented |
| G-7   | Alt-network keystore file perms race | LOW      | reviewed, mitigated |
| G-8   | Alt-network export plaintext on disk | MEDIUM   | reviewed, operator-doc'd |
| G-9   | Clock-drift fail-fatal as DoS vector | LOW      | reviewed, accepted |
| G-10  | Asset registry path traversal        | LOW      | reviewed, no action needed |
| G-11  | order_views permlink injection       | MEDIUM   | reviewed, mitigated |
| G-12  | order_views table size growth        | LOW      | reviewed, mitigated |
| G-13  | order_views public read disclosure   | LOW      | accepted tradeoff |
| G-14  | view-count POST as referer leakage   | LOW      | reviewed, no action needed |
| G-15  | degraded.html CSP + XSS              | LOW      | **fixed during audit** |
| G-16  | Order detail viewer-side timing      | LOW      | reviewed, no action needed |

**Result:** 0 HIGH, 2 MEDIUM (reviewed and accepted with
documented tradeoffs), 13 LOW (1 fixed during audit, 12
reviewed with no action needed).  No HIGH-severity issues
introduced by this campaign's changes.  All MEDIUM issues are
accepted tradeoffs documented in the relevant feature docs.

---

## Findings

### G-1 — `morphit-ops edit` parser robustness — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/ops-cli/src/commands/edit.ts` —
`parseKvLines` and `applyUpdates`.

**Threats considered:**
- Operator's existing `morphit.config.env` contains a malformed
  line.  Could the parser accept a line that should be rejected?
- Operator's file contains a key that's prefix-similar to one
  we want to edit (e.g., `MORPHIT_INSTANCE_ORIGIN_LEGACY=...`).
  Could `applyUpdates` accidentally replace it?

**Check:** `parseKvLines` finds `=` via `indexOf` and slices
`line.slice(0, eq).trim()` for the key.  Equality match in
`applyUpdates` is `key === ` against the exact key, not
prefix.  `MORPHIT_INSTANCE_ORIGIN_LEGACY` would not match
`MORPHIT_INSTANCE_ORIGIN`.

Quoted values:
- Lines like `KEY="value with spaces"` parsed correctly (strips
  outer quotes).
- Lines like `KEY=value with spaces` (no quotes, has spaces)
  return `value with spaces` (everything after `=`, trimmed) —
  technically not a valid env-var format but our writer never
  emits this, and reading-back doesn't matter for the editable
  fields (URLs, addresses, SEO copy don't contain spaces in
  practice — and even SEO copy is round-trip-safe via the
  always-quote-on-spaces rule in `quoteValue`).

**Action:** None.

---

### G-2 — `morphit-ops edit` quoting — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/ops-cli/src/commands/edit.ts` — `quoteValue`.

**Threats considered:**
- Operator pastes a malicious SEO description containing shell
  metacharacters or backslash-escapes.  Could the written file
  be exploited when sourced via `set -a; . morphit.config.env;
  set +a`?

**Check:** `quoteValue` uses double-quoted form for any value
that contains characters outside `[A-Za-z0-9_./:@\-+]`.
Backslashes are escaped (`\` → `\\`), then double-quotes (`"` →
`\"`).  Result: a value like `"$(rm -rf /)"` becomes
`"\"\$(rm -rf /)\""` — wait, we don't escape `$`.  Let me check.

Actually we don't escape `$` or backticks.  When the file is
sourced via `set -a; . morphit.config.env`, bash WILL evaluate
`$(...)` and backticks in double-quoted values.

**Mitigation already in place:** the values written go into
`process.env` for the indexer/relay, NOT into shell-evaluated
contexts at runtime.  The wizard's documented usage is also
`source morphit.env` for the critical-infra file, not
`morphit.config.env` (which is read via Node's `parseEnv` —
not bash).  `parseEnv` is purely textual; `$(...)` substrings
land in env vars verbatim.

But operators following a snippet that says "source
morphit.config.env" (some might) WOULD be exposed.  Let me
check: do we tell operators to source morphit.config.env
anywhere?

Searched `docs/`, `apps/ops-cli/src/init/render.ts`, and
`scripts/`: only `morphit.env` (the critical-infra file) is
documented for shell-sourcing.  `morphit.config.env` is
parseEnv-only, never bash-sourced.

**Action:** None.  But noting for future: if anyone ever
introduces a "source the operator config" path, harden the
quoter to escape `$` and backticks too.

---

### G-3 — `/v1/chain-fee` cache poisoning — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/indexer/src/api/chainFee.ts`.

**Threats considered:**
- A malicious chain RPC node returns a wildly wrong
  `account_creation_fee`, gets cached for 24h, frontend
  displays "10000 BLURT" — confuses users into thinking
  signups cost a fortune.

**Check:** The endpoint already has a fallback when chain RPC
is unreachable.  When chain RPC is REACHABLE but returns garbage,
the cache stores the garbage.  Our `fetchChainProperties` validates
the shape (throws if `account_creation_fee` is unparseable) — so
truly malformed responses fall through to the fallback path.

A more sophisticated attacker who controls one of the
operator's RPC endpoints could return a STRUCTURALLY VALID but
semantically wrong fee (e.g., 99 BLURT instead of 100).  The
worst case impact: frontend FAQ displays "currently 99 BLURT"
for up to 24h — operator's relay still validates the actual
chain value at signup time (it has its own 10% sanity threshold
per `MORPHIT_INDEXER_ACCOUNT_CREATION_FEE_BLURT`).  No fund
loss; cosmetic only.

**Action:** None.  The relay's runtime sanity check is the
real defense; the displayed value being slightly off is not a
high-severity issue.

---

### G-4 — `/v1/chain-fee` RPC hostile response — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/indexer/src/blurt/chainProperties.ts` (pre-
existing) called by `apps/indexer/src/api/chainFee.ts` (new).

**Threats considered:**
- Hostile RPC returns a response with prototype-pollution
  payload, e.g., `{"__proto__": {"foo": "bar"}}`.

**Check:** The RPC client uses `JSON.parse` (which doesn't
trigger setters and doesn't merge into Object.prototype).  The
indexer reads only specific top-level fields
(`account_creation_fee`).  No `Object.assign` or spread of the
raw response into another object.

**Action:** None.

---

### G-5 — SEO override storage XSS — MEDIUM

**Status:** reviewed, no action needed (Svelte auto-escaping +
CSP).

**Surface:** `apps/web/src/lib/components/Head.svelte` —
displays `$instance.seo.title`, `description`, `keywords`.

**Threats considered:**
- Operator (or whoever controls the operator's
  morphit.config.env) sets
  `MORPHIT_INSTANCE_SEO_TITLE='<script>steal()</script>'`.
  When users land on the homepage, does the browser execute it?

**Check:** Svelte's `<title>{title}</title>` and
`<meta name="description" content={description}>` use
auto-escaping; the value is set as a text node / attribute
value, never as innerHTML.  Even if the operator deliberately
crafts an XSS payload, the browser sees literal text.

CSP (configured in `svelte.config.js`) prohibits inline
`<script>` tags entirely — even if escaping somehow failed,
the script would be blocked at load.

Verified the JSON-LD path does NOT route operator-controlled
SEO override fields: `apps/web/src/lib/seo/jsonld.ts` builds
schemas from `siteName` (svelte-i18n `seo.site_name`) and
`tagline` (`app.tagline`), neither of which is operator-set.
The operator's `MORPHIT_INSTANCE_SEO_*` knobs flow only
through `Head.svelte`'s `<title>` / `<meta description>` /
`<meta keywords>` outputs, all of which use Svelte's
auto-escaping at the attribute level.

The operator IS the trust authority for their own instance,
though — if their config is compromised, lots of other things
are too (the chain-broadcast fee account, etc.).  This isn't
really a user-input-trust boundary; it's an operator-config-
trust boundary.

**Action:** None.

---

### G-6 — Alt-network keystore passphrase reuse — LOW

**Status:** accepted tradeoff, documented in `altKeystore.ts`
header.

**Surface:** `apps/ops-cli/src/init/altKeystore.ts` — same
passphrase encrypts the relay's posting key AND the alt-network
service keys.

**Threats considered:**
- An attacker who compromises the operator's passphrase via
  keylogger / social engineering can decrypt EVERY service key
  the operator runs.  No compartmentalization.

**Check:** This is a documented tradeoff (see file header lines
12-21).  The operator-UX value of "one passphrase per instance"
was explicitly chosen over compartmentalization.

Operators with stricter requirements can:
- Run the alt-network services in separate trust domains
  (separate hosts) where the passphrase is genuinely separate.
- Not encrypt these keys via Morphit at all (they're
  service-tool's own files — Tor stores them at
  `/var/lib/tor/...` and doesn't require Morphit's involvement).

**Action:** None — the tradeoff is documented in the source
file's header.

---

### G-7 — Alt-network keystore file perms race — LOW

**Status:** reviewed, mitigated.

**Surface:** `apps/ops-cli/src/commands/importAltnetKey.ts` —
the directory creation and the file write.

**Threats considered:**
- Between `mkdirSync(altDir)` and `chmodSync(altDir, 0o700)`,
  the directory exists at default umask (usually 0755 →
  group/other can read).  Window for an unprivileged local user
  to enumerate the file's existence.

**Check:** Read the code:

```
mkdirSync(altDir, { recursive: true });
chmodSync(altDir, 0o700);
// ... (then writeFileSync at 0o600 + chmod 0o600)
```

Hmm — the directory is created with default umask first, THEN
chmodded.  On a single-tenant VPS where the operator is the
only user, this race is benign.  On multi-tenant systems
(uncommon for a Morphit instance), there's a brief window.

The `writeFileSync` itself uses `mode: 0o600` directly (Node
respects mode at create time for new files), so the FILE never
exists at default umask — only the directory has the brief
race.

For most Morphit deployments (single-tenant VPS or home
server), this is fine.  Operators on multi-tenant hosts
shouldn't be running Morphit anyway — the indexer's database
credentials, the relay's posting key, and these alt-network
keys are all on the same disk; the threat model is "you trust
the host's other users" already.

**Mitigation:** Document in the import command's success
message that the operator should verify perms after import:

  `ls -la apps/relay/altnet/`

Already present implicitly via the "permissions set to 600"
output line.  Acceptable.

**Action:** None.  Could tighten by `mkdirSync` then `chmod`
in a single os.umask() block; not worth the complexity for the
risk class.

---

### G-8 — Alt-network export plaintext on disk — MEDIUM

**Status:** reviewed, operator-documented.

**Surface:** `apps/ops-cli/src/commands/exportAltnetKey.ts` —
`--out=PATH` writes plaintext to disk.

**Threats considered:**
- Operator runs `morphit-ops export-altnet-key
  --out=/some/path` to feed Tor at startup.  The plaintext
  sits at that path until something deletes it.  If the path
  is on persistent storage, the plaintext outlives the daemon's
  lifetime.
- A startup script that runs `--out=/var/lib/morphit/tor.key`
  and forgets to delete leaves the plaintext on disk
  indefinitely.

**Check:** The export command writes with `mode: 0o600` (good)
but does not delete after — that's the caller's job.  The
docstring notes "tmpfs is preferred" but doesn't enforce it.

**Mitigation:** Documented in the file's header:

  > Operators on a privacy-conscious system should prefer
  > tmpfs (`/dev/shm`, `/run/user/<uid>`) so the plaintext
  > never touches persistent disk.

This is the right place for the mitigation — within the
operator-side workflow, not in our code.  Forcing tmpfs from
within Morphit would be over-prescriptive; some operators have
legitimate reasons to write to other paths (Mode 0600 file in
their daemon's chroot, e.g.).

**Action:** None.  The header doc is the right
operator-facing guard.  Could tighten by adding a `--shred`
flag that overwrites the file before deletion; deferred to
post-launch if requested.

---

### G-9 — Clock-drift fail-fatal as DoS vector — LOW

**Status:** reviewed, accepted.

**Surface:** `apps/relay/src/main.ts` — relay refuses to start
if clock drift exceeds 120s.

**Threats considered:**
- A hostile chain RPC returns a fake `head_block_time` 5
  minutes in the future.  The relay's local clock is correct
  but the comparison flags it as drift > 120s and refuses to
  start.
- Multiple hostile RPCs do this in coordination — operator
  can't bring their relay up.

**Check:** The drift check happens AFTER the BlurtClient is
configured but BEFORE the listener.  RPC failure is silently
skipped (per the "RPC unreachable = silent skip" branch in
`main.ts`).  But a hostile-but-reachable RPC that returns lies
COULD prevent boot.

Mitigation: the BlurtClient rotates across multiple endpoints
configured by the operator.  Even one hostile endpoint among
several would cause inconsistent behavior (sometimes drift OK,
sometimes drift fatal).  Operators noticing this pattern should
remove the bad endpoint from their config.

For a fully-controlled adversary who replaced ALL of the
operator's RPC endpoints with malicious mirrors: they could
already do worse things than block boot (steal the operator's
posting key via a forged signed-broadcast response, etc.).
Boot-blocking is a low-impact addition to a much larger
compromise.

**Action:** None.  The drift check is a sanity net for the
common case (operator forgot to enable systemd-timesyncd),
not a defense against adversarial RPC.

---

### G-10 — Asset registry path traversal — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/web/src/lib/assets/registry.ts` — `logoSvgPath`.

**Threats considered:**
- Could a future entry include `../../etc/passwd` for
  `logoSvgPath` and trick the frontend into rendering it?

**Check:** The registry is operator-controlled at build time
(it's compiled into the JS bundle, not loaded at runtime).
A malicious entry could only be added by someone who has
push access to the Morphit codebase.  At that point they
control the entire frontend; path traversal in an SVG path is
not the largest concern.

The smoke test enforces `/coins/` prefix and `.svg` suffix as
a contract:

```
if (!p.startsWith('/coins/')) {
    throw new Error(...);
}
```

This catches accidental developer typos (`./coins/btc.svg`)
that wouldn't resolve correctly anyway.

**Action:** None.

---

### G-11 — order_views permlink injection — MEDIUM

**Status:** reviewed, mitigated.

**Surface:** `apps/indexer/src/api/orderViewsLogic.ts`.

**Threats considered:**
- A user crafts a permlink containing SQL injection or
  parameter-pollution characters.  Could the parameterized
  query be subverted?
- A user crafts an extremely long permlink to cause memory
  blow-up.

**Check:** Both endpoints validate `account` and `permlink`
with regex:

```
const PERMLINK_RE = /^[a-z0-9-]+$/;
function isValidPermlink(s) {
    if (typeof s !== 'string') return false;
    if (s.length === 0 || s.length > 256) return false;
    return PERMLINK_RE.test(s);
}
```

256-char cap blocks oversize attacks.  Regex restricts to
`[a-z0-9-]` — no quotes, no escapes, no SQL metacharacters.
The query uses parameterized substitution (`$1` placeholder)
even after validation; defense in depth.

**Action:** None — already mitigated at validation + query
layer.

---

### G-12 — order_views table size growth — LOW

**Status:** reviewed, mitigated.

**Surface:** `apps/indexer/src/db/schema-v22.sql`.

**Threats considered:**
- An attacker creates many fake orders and bumps their view
  counts to bloat the `order_views` table.

**Check:** The increment endpoint refuses to create counters
for non-existent orders (`SELECT EXISTS(SELECT 1 FROM orders
WHERE account = $1 AND permlink = $2)`).  No order = no
counter row.  The order itself costs the listing fee to
create — at 60 BLURT × 1000 fake orders = 60,000 BLURT to
generate 1000 useless rows.  Attack is uneconomic.

Spam at scale is also rate-limited at nginx.

**Action:** None — already mitigated by the existence check.

---

### G-13 — order_views public read disclosure — LOW

**Status:** accepted tradeoff, documented.

**Surface:** GET /v1/orders/:account/:permlink/views is
public-readable.

**Threats considered:**
- A scraper hits GET for every permlink they can find,
  building a popularity dataset they can then use to target
  high-interest orders for fraud or surveillance.

**Check:** The threat is real but bounded.  A scraper could
already do the same thing by sitting on the orderbook page and
counting clicks.  The count itself is non-identifying (no IP,
no viewer account, no timestamps per view).  See the privacy
notes in the orderViewsLogic.ts header.

The display is gated client-side: only the order's author sees
the count.  But anyone CAN call the endpoint and read the
count.  This is the structural privacy tradeoff documented in
the audit's design phase: signature-gating GET would require
bundling secp256k1 verify in the indexer for one soft metric.
Not worth it.

**Action:** None.  The privacy property is structural (count
alone is non-identifying) rather than gate-enforced.

---

### G-14 — view-count POST as referer leakage — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/web/src/lib/orders/views.ts` —
`recordOrderView`.

**Threats considered:**
- The frontend POSTs to `/v1/orders/:account/:permlink/view`
  when the user clicks "Message".  Does the request leak
  anything beyond what a normal indexer hit would?

**Check:** The fetch is `credentials: 'omit'` — no cookies, no
Authorization header.  Same-origin in production.  Referer is
the order's permlink path which the indexer already knows
about (it's the order they're inspecting).  No new leak.

**Action:** None.

---

### G-15 — degraded.html CSP compliance + operator data leakage — LOW

**Status:** reviewed, **fixed during audit**.

**Surface:** `apps/web/static/degraded.html` and
`apps/web/static/degraded.css`.

**Threats considered:**
- Original draft of degraded.html had an inline `<script>` that
  fetched `/v1/instance` for branding, plus an inline `<style>`
  block.  The operator's recommended CSP (`script-src: self;
  style-src: self`, see OPERATIONS.md §15) would block both —
  the page would render unstyled and silently fail to fetch
  branding in any reasonably-configured deployment.
- Even if CSP didn't block them, the script's `innerHTML`-via-
  `escapeHtml`-string-concatenation was a small XSS surface
  for a malicious operator-set instance name.

**Check:** Discovered during audit re-walk of the new files.
Inline `<script>` violates `script-src: self`; inline `<style>`
violates `style-src: self`.

**Fix:**
- Removed the inline `<script>` entirely.  Degraded mode now
  shows generic Morphit branding (no operator name fetched).
  Honest framing: in degraded mode, operator-specific copy is
  the wrong priority anyway — the goal is "tell users this is
  load-shedding, point them somewhere useful, get out of the
  way."
- Moved styles to `apps/web/static/degraded.css`.  External
  stylesheet, served by nginx alongside `degraded.html`,
  CSP-clean under `style-src: self`.
- Updated OPERATIONS.md to allowlist `/degraded.css` in the
  reachable-during-degraded routes block.

**Action:** Fixed.  Verified the page renders correctly without
any inline content; both files are pure static.

---

### G-16 — Order detail viewer-side timing — LOW

**Status:** reviewed, no action needed.

**Surface:** `apps/web/src/routes/my/orders/+page.svelte` —
viewcounts loaded with `Promise.all` over all visible items.

**Threats considered:**
- The view-count fetches reveal which orders the page-owner
  has via timing analysis.  But this is the AUTHOR's view of
  THEIR OWN orders — they already know which orders they have.

**Check:** No new disclosure surface.  The fetches go to the
same indexer the page is already talking to.

**Action:** None.

---

## Summary by severity

- **HIGH:** 0 introduced.
- **MEDIUM:** 2 introduced, both reviewed and accepted with
  documented tradeoffs (G-5 operator-trust boundary for SEO
  override, G-13 public-readable view counts as structural
  privacy property).
- **LOW:** 13 reviewed: 1 fixed during audit (G-15 — CSP
  compliance for degraded.html, accomplished by externalizing
  styles and dropping the unused branding-fetch script), 12
  non-issues or already-mitigated.

## Recommendation

Cleared to proceed to Phase G (mobile PWA polish + final
pre-launch deep-audit pass).  No HIGH-severity vulnerabilities
introduced by the post-F.5 task campaign.

The two MEDIUM findings (G-5, G-13) are documented design
tradeoffs.  G-5 (operator-set SEO copy XSS): trust boundary is
correctly drawn — the operator is the authority for their own
instance, and Svelte auto-escaping plus CSP defense-in-depth
holds.  G-13 (public-readable view counts): structural privacy
property holds (count alone is non-identifying) and the
alternative (secp256k1 verify in indexer) was deemed
disproportionate to the soft-metric value.