security: add secret-scan CI gate + persisted-key allowlist tripwire (#163)
An unsolicited scanner reported CRITICAL "LLM output written to a persistent memory store" findings against src/lib/notifications.ts:145, src/lib/sdk.ts:392 and src/lib/session-grouping.ts:24. All three are false positives: the cited lines are an in-memory notification dedupe Map, a URLSearchParams limit param, and a bucket push inside a pure grouping helper. The app persists nothing model-derived — sessions and messages live on the server and are held in memory by the stores. Two gates so that stays true and so the one class of report that WAS real for us (credentials in git history) gets caught before a push: - security-scan.yml: gitleaks on push/PR to main, full history fetch. - persisted-keys.test.ts: enumerates every SecureStore write by key. A new persistence sink fails the suite until someone adds the key with a note saying what it holds — which is the moment to notice if it's model output rather than user config. Verified it trips by adding a throwaway "cache the assistant reply" write. Co-authored-by: engineer <engineer@macbookpro.lan> Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
31
.github/workflows/security-scan.yml
vendored
Normal file
31
.github/workflows/security-scan.yml
vendored
Normal file
@@ -0,0 +1,31 @@
|
||||
name: Security scan
|
||||
|
||||
# Why this exists: the one accurate external report we ever received was
|
||||
# Postgres credentials committed to public repo history. The finding itself
|
||||
# was cheap to fix; what was missing was a gate that would have caught it
|
||||
# before the push. This is that gate.
|
||||
|
||||
on:
|
||||
push:
|
||||
branches: [main]
|
||||
pull_request:
|
||||
branches: [main]
|
||||
|
||||
permissions:
|
||||
contents: read
|
||||
|
||||
jobs:
|
||||
secrets:
|
||||
name: Secret scan (gitleaks)
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v6
|
||||
with:
|
||||
# Full history so a secret introduced in an earlier commit on the
|
||||
# branch is caught, not just the tip diff.
|
||||
fetch-depth: 0
|
||||
|
||||
- name: gitleaks
|
||||
uses: gitleaks/gitleaks-action@v2
|
||||
env:
|
||||
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
|
||||
84
src/lib/persisted-keys.test.ts
Normal file
84
src/lib/persisted-keys.test.ts
Normal file
@@ -0,0 +1,84 @@
|
||||
import { test } from "node:test"
|
||||
import assert from "node:assert/strict"
|
||||
import fs from "node:fs"
|
||||
import path from "node:path"
|
||||
|
||||
/**
|
||||
* Tripwire for the "poisoned persistent memory" class of bug (a scanner
|
||||
* flagged us for it; see the triage in the security notes).
|
||||
*
|
||||
* The app is a thin client: sessions, messages and tool output live on the
|
||||
* opencode server and are held in memory by the zustand stores. Nothing that
|
||||
* a model or a tool produced is ever written to on-device storage, so there
|
||||
* is no persistent store an injected instruction could sit in and be replayed
|
||||
* from on the next launch.
|
||||
*
|
||||
* That property is worth keeping, and it is invisible in review: it only
|
||||
* breaks the day someone adds a well-meaning "cache the last conversation" or
|
||||
* "remember the assistant's summary" write. So: every on-device write is
|
||||
* enumerated here by key. Adding a new one is fine — you just have to come
|
||||
* to this list and say what it holds, which is the moment to ask whether the
|
||||
* value is user/config data or model output.
|
||||
*
|
||||
* Rule: keys listed here must hold user-entered config, consent flags or
|
||||
* counters. Never message text, session titles, tool results or any other
|
||||
* model-derived content.
|
||||
*/
|
||||
const ALLOWED_PERSISTED_KEYS = new Map<string, string>([
|
||||
["SETTINGS_KEY", "user's app settings (theme, notification prefs)"],
|
||||
["CONNECTIONS_KEY", "user-entered server connections"],
|
||||
["`${PASSWORDS_PREFIX}${id}`", "user-entered server password, per connection"],
|
||||
["RECENT_DIRS_KEY", "directories the user picked, for the recents list"],
|
||||
["AUTH_SETTINGS_KEY", "biometric/app-lock preference"],
|
||||
["COUNT_KEY", "store-review: launch counter"],
|
||||
["ASKED_KEY", "store-review: already-prompted flag"],
|
||||
["FIRST_OPEN_KEY", "analytics: first-open flag"],
|
||||
["CONSENT_KEY", "telemetry consent decision"],
|
||||
["CHATWOOT_SOURCE_KEY", "support contact id issued by Chatwoot"],
|
||||
])
|
||||
|
||||
const SRC = path.join(import.meta.dirname, "..")
|
||||
|
||||
function sourceFiles(dir: string): string[] {
|
||||
return fs.readdirSync(dir, { withFileTypes: true }).flatMap((entry) => {
|
||||
const full = path.join(dir, entry.name)
|
||||
if (entry.isDirectory()) return sourceFiles(full)
|
||||
if (!/\.tsx?$/.test(entry.name)) return []
|
||||
if (entry.name.includes(".test.")) return []
|
||||
return [full]
|
||||
})
|
||||
}
|
||||
|
||||
test("every on-device write uses a reviewed key, never model-generated content", () => {
|
||||
const unknown: string[] = []
|
||||
|
||||
for (const file of sourceFiles(SRC)) {
|
||||
const code = fs.readFileSync(file, "utf8")
|
||||
for (const match of code.matchAll(/SecureStore\.setItemAsync\(\s*([^,]+?)\s*,/g)) {
|
||||
const key = match[1].trim()
|
||||
if (!ALLOWED_PERSISTED_KEYS.has(key)) {
|
||||
unknown.push(`${path.relative(SRC, file)}: ${key}`)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
assert.deepEqual(
|
||||
unknown,
|
||||
[],
|
||||
`New persistent write(s) found. If the value is user config, add the key to ALLOWED_PERSISTED_KEYS ` +
|
||||
`with a note. If it is model or tool output, don't persist it — it comes back as context next launch:\n` +
|
||||
unknown.join("\n"),
|
||||
)
|
||||
})
|
||||
|
||||
test("the allowlist itself stays live (no keys left behind after a refactor)", () => {
|
||||
const code = sourceFiles(SRC)
|
||||
.map((file) => fs.readFileSync(file, "utf8"))
|
||||
.join("\n")
|
||||
const used = new Set(
|
||||
[...code.matchAll(/SecureStore\.setItemAsync\(\s*([^,]+?)\s*,/g)].map((match) => match[1].trim()),
|
||||
)
|
||||
const stale = [...ALLOWED_PERSISTED_KEYS.keys()].filter((key) => !used.has(key))
|
||||
|
||||
assert.deepEqual(stale, [], `Allowlist entries no longer written anywhere — delete them:\n${stale.join("\n")}`)
|
||||
})
|
||||
Reference in New Issue
Block a user