Portfolio Inbox: Twelve Bugs Found by Execution, Not by Reading
Hardening the Portfolio contact inbox (Pages Function → private GitHub repo → local mirror + append-only archive) surfaced 12 defects, of which 10 were found only by running the code, not by reading it. Two mattered most: (a) String(undefined) is the literal string "undefined", so a submission that omitted a required field passed the falsy-value guard and was permanently archived under that name; (b) the archive's markdown block parser could be made to archive a forged message by a visitor, because the sanitizer escaped one delimiter character but left line structure intact, and my first regex accidentally blocked the obvious attack while leaving a precision-variant wide open. A recurring second class was wrong diagnosis — an edge-rule rejection, a missing JSON field, and an absent-but-tolerated file were each reported as credential failures. Headline lesson: a test that fails because the test is wrong is still information — my one bad assertion is what exposed the injection hole.
Redaction note (2026-09-28). The first public version of this entry included operational specifics that doubled as attack aids. They have been removed, and this copy deliberately does not describe which ones — an enumeration would itself confirm the shape of the attack surface. The bugs, the counts, and the lessons stand on their own; none of them needed the recipes. The local session record retains the full detail.
2. Background & Problem Statement
The contact form does not email. The Function validates, then performs a full PUT rewrite of a markdown file in a private GitHub repo, prepending the new block. A separate maintenance endpoint truncates that file back to an empty placeholder. So the remote file is a snapshot, not a log — a dated history proves it: eight messages exist only in git history, absent from the file.
Three properties had to hold simultaneously:
- Loss-free. A remote reset must never destroy a message.
- No false negatives. A new message must be detected exactly once.
- No false positives. A forged or malformed payload must never be archived or reported as a real message.
Plus a credential preflight, because the failure mode nobody sees is the Platform secret dying silently: visitors get an error and simply give up.
3. Environment / Setup
| Component | Detail |
|---|---|
| Site | Cloudflare Pages (project hashbyte), Functions on the Workers runtime |
| Runtime constraints | V8 isolate, fetch only — no shell, no ssh, no binaries |
| Store | one markdown file in a private GitHub repo, default branch main |
| Write credential | a Pages secret (bearer token) held by the Function |
| Read credential | git over SSH, run locally |
| Read fallback | an authenticated GitHub CLI token on machines with no SSH agent |
| Gate credential | a second Pages secret gating the two maintenance endpoints |
| Toolchain | Node v26.8.2, Python 3.14.7, gh 2.100.0 |
| Test harness | 5 suites, 92 assertions (12 / 3 / 22 / 27 / 28) |
3.1 SSH vs token — the question that clarified the whole design
"I connect to the repo using SSH — why would I need a token?"
Two unrelated authentication systems:
| authenticates | usable for | |
|---|---|---|
| SSH key | the git protocol | clone / fetch / push only |
| OAuth / PAT token | the REST API | CLI tooling, and the Function |
SSH keys are meaningless to the REST API — an unauthenticated request for a private repo returns 404, not 403, precisely so the repo's existence isn't confirmed. But the token is not for us; it is for the platform host. The Workers runtime cannot do SSH under any configuration, and handing the host an SSH key to write one repo would hand it write access to every repo that key can reach. A token scoped to a single repo is strictly safer. The token is the correct design, not a workaround.
Also established: a passphrase-protected key with no running agent fails Permission denied (publickey) in cron / launchd / CI, so unattended reads need the keychain unlocked first.
4. Investigation / Method
Bugs were classified by how they were found, because that is the transferable part:
- E = found by executing code (unit/integration test or live probe)
- R = found by re-reading before deploy
| # | Bug | Class | Found by |
|---|---|---|---|
| 1 | String(undefined) → "undefined", passes a falsy guard | injection/validation | E (live probe) |
| 2 | Fence-unaware parser archived a forged block | injection | E (test) |
| 3 | Edge rule returned 403 on the default client UA, reported as a credential fault | wrong diagnosis | E (live) |
| 4 | CLI auth status collapsed a 401 into "not logged in" | wrong diagnosis | E (test) |
| 5 | Scope regex captured only the first of several scopes | wrong diagnosis | E (live) |
| 6 | Absent field treated as false → "secret missing" for a read-only token | wrong diagnosis | E (test) |
| 7 | Absent target file reported unhealthy (false alarm) | wrong diagnosis | E (test) |
| 8 | GET unusable as a liveness signal — Pages serves the static site | wrong diagnosis | E (live) |
| 9 | Non-JSON 200 → confusing unexpected HTTP 200 | wrong diagnosis | E (test) |
| 10 | Error-body read inside the exception handler can itself raise | crash | E (test) |
| 11 | Within-batch duplicates archived twice; across-sync collapsed once | logic | E (test) |
| 12 | Maintenance endpoint's two 500s swapped during refactor | regression | R |
5. Experiments & Results — with Evidence
5.1 Bug 1 — String(undefined) is not empty
String(undefined) === "undefined" and String(null) === "null", so every sanitizer returned a truthy string for an absent field. The combined validator was accidentally safe only because that literal fails both the email and phone regexes.
$ node -e "import('./functions/api/_shared.js').then(m=>{
console.log('sanitizeName(undefined) =', JSON.stringify(m.sanitizeName(undefined)));
console.log('validateContact(undefined) =', JSON.stringify(m.validateContact(undefined))); })"
sanitizeName(undefined) = "undefined"
validateContact(undefined) = null
# Post with the name field OMITTED, contact + message valid:
# name -> "undefined" (truthy, so the `!name` guard does NOT fire)
# contact -> "real@example.com"
# message -> "hello"
# => ACCEPTED, archived with name "undefined"Surfaced by noticing the live endpoint's error text. An empty post returned the contact error rather than the name error — proof the name guard had not fired:
$ curl -s -X POST https://hashbyte.pages.dev/api/contact \
-H 'content-type: application/json' -d '{}'
{"error":"that email/phone does not look right"} # expected: "a proper name is required"Fix: a coercion helper mapping undefined/null → '' in all three sanitizers, so the required-field guards fire. 13 assertions; normal values unchanged.
This matters because the archive is append-only: a garbage entry can no longer be cleanly removed later.
5.2 Bug 2 — forged block injection (the near-miss)
Block identity is taken from a level-2 markdown heading carrying an ISO timestamp, matching what the writer appends. The sanitizer strips control characters and escapes the code-fence delimiter, but leaves line structure and heading markers untouched, so a visitor controls arbitrary lines inside the fence.
The first cut identified blocks by regex alone. It appeared to work, because the natural attack string — a far-future timestamp with no sub-second component — failed to match:
attacker body line: heading + far-future timestamp, no sub-second part
-> does NOT match (the pattern requires them) -> "safe"That was false confidence. Adding the sub-second part made it match perfectly:
$ python3 -c "...parse_blocks(body)..."
blocks parsed: 2 (expected 1)
- the genuine block
- a heading the visitor invented <-- attacker-controlled, archived as realFix: fence-aware parsing — a heading only opens a block when not inside the message fence. The payload still round-trips verbatim inside the genuine message. The fence is reliable precisely because its delimiter is escaped, so an unescaped delimiter can only be the block's own.
How it was found — the part worth keeping. My first regression test asserted the archive must not contain the string Forged. It failed. The obvious read was "my parser is broken"; the correct read was "my assertion is wrong — Forged legitimately appears inside the code fence as payload". Fixing the bad assertion prompted testing the precision variant, which is where the real hole was. A failing test is sometimes the test's fault, but it is always information.
5.3 Bug 3 — the edge rule impersonates a credential fault
An edge rule refuses the default urllib user-agent outright, so the first version of the preflight reported that rejection as a broken token — sending the user to rotate a perfectly valid credential. It never reached the Function, so it learned nothing about the token at all.
# pre-fix: default client UA -> 403 (edge refused; secret never tested)
# post-fix: app-specific UA -> 400 (reached the Function's own validation)Fixed by pinning an explicit user-agent, and by reporting a 403 as an edge block, never a token fault. Asserted as a permanent regression guard.
5.4 Bug 8 — GET cannot detect a missing Function
GET /api/health (undeployed) -> HTTP 200 type=text/html <-- the static site
POST /api/health (undeployed) -> HTTP 405 <-- clean signal
POST /api/contact (deployed) -> HTTP 400 type=application/jsonA GET is unusable as a liveness signal: Pages serves the static asset for an undeployed Function path, and 200 text/html is indistinguishable from a real answer. A POST to a missing Function is a clean 405. The client therefore POSTs, and on 405 — or a 200 that is not JSON — falls back to the weaker submission probe and reports validity UNPROVEN rather than quietly passing.
5.5 Bugs 6 & 7 — two false diagnoses in the health check
- The remedy dispatch tested "configured is falsy" first. A response that merely omitted the field satisfied that test → "secret missing" reported for a token that was merely read-only. Fixed: dispatch on the failure text first, and compare with
is False. - The target file returning
404was scored unhealthy. It is not a fault: the read path maps 404 to an empty placeholder and the write path recreates the file, so writing still works. Now reported asfile: "absent"withok: true.
5.6 Verifying write capability without writing
The API has no "can I write here?" call, but the authenticated repo payload carries the token holder's own permissions. So a push permission flag proves a write would be accepted — two GETs, zero mutations:
--- it must never write ---
PASS zero non-GET calls
PASS exactly 2 GETs (repo + contents)
--- cache: cannot be used to burn the token's rate limit ---
PASS 25 calls -> still only 2 API calls
PASS cached responses are markedHardened for a public site: gated behind the shared secret; booleans and status codes only, never the token and never an upstream body (a raw pass-through could leak token detail inside an error payload); 60s per-isolate cache.
5.7 Bug 12 — caught by re-reading, not by a test
Refactoring the maintenance endpoint onto a shared guard helper silently swapped the order of its two 500s. Only reachable when both secrets are unset, so no happy-path test would notice. Caught by diffing against the original before deploying, and preserved exactly by splitting the "is it configured" check into its own helper.
6. Conclusions
- Execution finds bugs that reading does not. 10 of 12 defects were invisible to inspection. Both critical ones (
String(undefined), forged-header injection) look correct in a code review. - A passing test is not a safe test. The precision requirement in the block regex accidentally blocked the obvious attack and nearly shipped. The security property needed was "a heading inside a fence is never a heading" — expressed as fence tracking, not as a stricter regex.
- A wrong diagnosis is worse than no diagnosis. Six bugs mis-pointed the user at the credential when the fault was elsewhere (edge rule, absent field, tolerated 404, missing Function, static fallthrough). Each is now asserted.
- Probe the thing that actually fails. A CLI
auth statuscall also exits non-zero on a revoked token, so the API probe must run first and status is used only to enrich a pass. - When a fallback is weaker, say so in the output.
validity UNPROVENis honest; a green check that only proved presence would have been a lie. - Treat append-only archives as write-once. A malformed entry can never be cleanly removed, so validation must reject at ingest rather than clean later.
- Publishing a postmortem is a second, separate disclosure. Operational specifics — bypass strings, secret names, file locations, parser formats — are attack aids even when the write-up is about fixing them. Redact the recipes, keep the reasoning.
7. Final Configuration
| Concern | Decision |
|---|---|
| Local read transport | git over SSH (no token); --transport api for keyless machines |
| Working copy | data files are siblings of the clone, so no git reset/clean can clobber the archive |
| Mirror | whatever git checked out; hand edits self-correct on next pull |
| Newness | sha256 of the whole block, not position or commit |
| Write-ability check | authenticated repo metadata → push permission; zero writes |
| Absent target file | ok: true — the write path recreates it |
| Absent field | coerced to '' so required-field guards fire |
| Health liveness | POST (not GET); 405 ⇒ not deployed ⇒ fall back and say UNPROVEN |
| Sensitive endpoints | gated on a shared secret; 60s cache; booleans only |
| Auto-renewal | not possible — a platform secret is set by hand and redeployed. An app-installation token is the only self-renewing design. |
Key files
functions/api/health.js— read-only token health checkfunctions/api/_shared.js— coercion helper, fence-safe contract, shared guardscripts/inbox/sync_inbox.py— SSH/API transport, hash-dedupe archive, preflightinbox/— mirror, archive, and a git working copy (gitignored: visitor PII)
Deployed 2026-09-28 (all three Functions, plus the coercion fix):
npx wrangler pages deploy . --project-name hashbyte --commit-dirty=true8. Appendix — Reproduction
# 1. String(undefined) coercion (bug 1)
cd /Volumes/Xbyte/Projects/Portfolio/functions/api
node --input-type=module -e "
import { sanitizeName, validateContact } from './_shared.js';
console.log(JSON.stringify(sanitizeName(undefined)), JSON.stringify(validateContact(undefined)));"
# pre-fix: "undefined" null post-fix: "" null
# 2. live endpoint before the fix showed the un-guarded name
curl -s -X POST https://hashbyte.pages.dev/api/contact \
-H 'content-type: application/json' -d '{}'
# {"error":"that email/phone does not look right"} <- should be "a proper name is required"
# 3. SSH unusable non-interactively
ssh-add -l # The agent has no identities.
# -> Permission denied (publickey); unlock the keychain, or use --transport api
# 4. full suites (92 assertions)
cd /Volumes/Xbyte/Projects/Portfolio
python3 /tmp/xgcode/test_sync.py && python3 /tmp/xgcode/test_check.py \
&& python3 /tmp/xgcode/test_git.py && python3 /tmp/xgcode/test_health_client.py \
&& node /tmp/xgcode/test_health.mjs
# 5. preflight, both transports
./scripts/inbox/pull.sh --check-only # ssh
./scripts/inbox/pull.sh --transport api --check-only # CLI tokenOn omissions. Some details present in the working notes were left out of this public copy. Which details is itself information, so they are not listed here; the local session record retains them.