Skip to content

feat: iBamboo admin POC control plane - #51

Merged
SteveSimonson merged 2 commits into
mainfrom
feature/ibamboo-admin-poc
Aug 1, 2026
Merged

feat: iBamboo admin POC control plane#51
SteveSimonson merged 2 commits into
mainfrom
feature/ibamboo-admin-poc

Conversation

@SteveSimonson

Copy link
Copy Markdown
Owner

Fixes #50

Summary

Proof-of-concept admin at https://ibamboo.com/admin so we can operate house controls before network roll-out.

Tab Capability
Overview Feature flags (flash / Conbal / quiz CRM)
Flash catalog Live product count + sample + link to Flash Catalog admin
Product library kyasi.us stats + ASIN list for site ibamboo
Conbal Origin, site key, health probe
Avatars Edit/add/remove house personas (KV)
Editor in Chief System prompt, demeanor, audience, language, guardrails, targeting categories

Auth: ADMIN_PASSWORD secret → HMAC session cookie (12h).
State: KV binding ADMIN_KV (ibamboo-admin).
API: worker/admin.ts/api/admin/*.

Out of scope (follow-ups)

  • Runtime storefront still reads avatars from vibes.ts until we wire KV
  • Feature flags not yet enforced on client
  • Full flash CRUD remains in Flash Catalog (GitHub OAuth)

Risk

Low–medium: new routes + KV + secret; no change to public merch paths unless someone hits /admin. Password secret must be set post-merge.

Test plan

  • npm run build pass
  • npm test 11/11
  • npm run lint clean
  • After merge: wrangler secret put ADMIN_PASSWORD + deploy
  • Login at /admin, save Editor config, probe flash/library/conbal

Password-gated /admin with flash catalog review, kyasi library
connect, Conbal probe, avatars manager, and Editor-in-Chief
prompt/categories stored in ADMIN_KV. Refs #50.
@SteveSimonson

Copy link
Copy Markdown
Owner Author

Independent review — PR #51 (feature/ibamboo-admin-poc)

Reviewed full diff + spot-checked worker/admin.ts, src/pages/Admin.tsx, worker/index.ts integration, and wrangler.jsonc ADMIN_KV binding. Local npm test 11/11 and npm run build pass.

What looks solid

Auth / session

  • Login requires ADMIN_PASSWORD Worker secret; missing secret → 503 (not a soft-open).
  • Session is HMAC-SHA-256 over admin:<exp>, cookie ibamboo_admin_session with HttpOnly, SameSite=Lax, Secure when https:, Max-Age=43200 (12h).
  • isAuthed correctly gates config GET/PUT and flash/library/conbal status; login / logout / session / health stay public as expected.
  • No secrets committed; only wrangler comments for secret put. KV namespace id is not a secret. Conbal site key documented as public embed id.

KV config merge

  • Load path: defaults ⟂ shallow merge nested objects; avatars replace-or-default; corrupt JSON → defaults.
  • PUT path: same nested merge pattern, forces version: 1, stamps updatedAt. Sane for a single-key POC config blob.

Error handling

  • Flash / library / conbal probes wrap fetch + json() in try/catch on the worker; SPA surfaces { ok: false, error } without blowing the shell.
  • credentials: 'include' on the admin api() helper matches cookie auth.

XSS / SEO

  • No dangerouslySetInnerHTML; config fields render as React text / form values (escaped).
  • /admin is not in routeMeta → worker serves SPA shell with noindex,nofollow (and currently HTTP 404). Accidental SEO containment is fine for this POC; robots hard-requirement not needed yet.

Integration

  • handleAdmin is wired before the generic /api/* 404 in worker/index.ts.
  • ADMIN_KV binding present in wrangler.jsonc; AdminEnv extended on WorkerEnv / env.d.ts.

Findings (adversarial-lite)

Security — nits / follow-ups (not merge blockers for POC)

  1. Password + HMAC compare are not constant-time (!== string eq). Fine for house POC; use crypto.subtle.timingSafeEqual (or equivalent) if this ever faces the public internet with a guessable password.
  2. No login rate limiting / lockout. Brute-force against a single shared password is the realistic threat model. Follow-up: CF rate-limit rule or simple KV-backed attempt counter.
  3. /api/admin/health is unauthenticated and discloses passwordConfigured + kv binding. Low sensitivity; still prefer auth or strip fields before production hardening.
  4. Logout Set-Cookie omits Secure (login sets it when HTTPS). Clear-cookie on HTTPS should also include Secure (and ideally match Path/SameSite) so the browser reliably drops the session cookie.
  5. Authenticated SSRF surface: flash catalogUrl, library baseUrl, and conbal origin are admin-writable and server-fetched. Acceptable with a single trusted operator; if multi-user later, allowlist schemes/hosts.
  6. Open-URL nits in SPA: href={config.flash.adminUrl} / href={config.library.baseUrl} and img src={av.image} trust stored config. Prefer https:-only allowlist on save (or when rendering) so a poisoned KV value cannot become javascript: / unexpected schemes. React text XSS is not the concern here.
  7. Misleading comment on flash status: // Live flash catalog snapshot (public GET) — endpoint is behind auth. Fix comment only.

Correctness

  1. /admin HTTP status is 404 (unknown path → serveShell(..., null, 404)). SPA still boots; operators see 404 in Network. Prefer an explicit allowlist for /admin200 + noindex (or a tiny routeMeta entry) so the control plane does not look broken. Not a functional blocker.
  2. Config PUT has no schema / size validation — arbitrary keys from ...body and unbounded JSON can land in KV. OK for single-operator POC; add a strip/allowlist before multi-writer use.
  3. Feature flags / avatars / editor config are storage-only until runtime wiring (called out in PR). No correctness bug today; just do not expect storefront behavior to change post-merge without the follow-up.

Maintainability

  1. AdminConfig types are duplicated in worker/admin.ts and src/pages/Admin.tsx. Drift risk; share a small shared type module when you next touch either side.
  2. Admin UI is not code-split — lands in the main ~752 kB client chunk. Lazy React.lazy(() => import('./pages/Admin')) would keep the storefront bundle cleaner.
  3. LIBRARY_TOKEN is declared on env but unused in admin routes (library probe is unauthenticated public GETs). Harmless dead wiring; remove or use when library requires auth.

Post-merge ops (from PR — still required)

  • npx wrangler secret put ADMIN_PASSWORD
  • Optional: ADMIN_SESSION_SECRET (recommended so rotating the login password does not invalidate the signing key in a confusing way — or does, intentionally; either way, set it deliberately)
  • Deploy + exercise login / save Editor / flash·library·conbal probes

Summary

Clean, well-scoped POC: auth boundary is in the right place, KV merge is coherent, no secrets in tree, probes fail closed enough for operators, and public merch paths are untouched. Remaining items are hardening and polish (rate limits, constant-time compare, /admin 200+noindex, lazy route, type sharing, runtime flag wiring) — appropriate as follow-ups, not as change requests for this PR.

VERDICT: APPROVE_WITH_NITS

Address review nits on PR #51: serve admin shell as 200+noindex,
match logout cookie Secure flag, lazy-load Admin chunk, fix flash
comment.
@SteveSimonson
SteveSimonson merged commit 7371e86 into main Aug 1, 2026
1 check passed
@SteveSimonson
SteveSimonson deleted the feature/ibamboo-admin-poc branch August 1, 2026 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

iBamboo admin POC control plane

1 participant