v1.0 readiness: governance docs, CI test gate, and security hardening (M1/M7) - #113
Merged
Conversation
…overlay gotcha - Add SECURITY.md: private disclosure path, supported versions, known limitations. - Add docs/versioning-policy.md defining what 1.0 SemVer covers (CLI + documented env vars are public; HTTP/SSE API, on-disk state, and experimental features are internal/unstable). - LICENSE: '2024 Claudeman Contributors' -> '2024-2026 Codeman Contributors'. - CLAUDE.md: fix the stale xterm-zerolag-input 'duplicated in app.js' gotcha (it is single-source now -> gitignored vendor bundle via postinstall.js/build.mjs); add versioning + security pointers; minor /init nav fixes (image-input load order, server.ts marker). - README: link SECURITY.md + the versioning policy. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…existing test debt - CI: add a 'test' job running the unit suite via config/vitest.ci.config.ts. Excludes browser (Playwright/chromium) and perf tests (timing-flaky), like the existing test/mobile suite. Safe in CI: TmuxManager no-ops shell commands under VITEST (test/setup.ts). - Add scripts/check-frontend-syntax.mjs (node --check on src/web/public/*.js), wired into the lint job — catches a class of frontend SyntaxError that passes lint today (lint globs only TS). - Add test/security-regression.test.ts (wired Host/Origin guard, self-update CSRF, CSP/security headers, text/plain raw body, WS anti-CSWSH) + test/sse-registry-parity.test.ts (backend<->frontend SSE registry parity). - Green pre-existing test debt surfaced by the new gate: stale 'Session not found' asserts -> 'not found' substring; drop tests for removed helpers (isError now internal; createSuccessResponse deleted); file-stream-manager: mock realpathSync + fix stale /tmp assertion; sse-subscription-filter: lifecycle events broadcast to all clients (only terminal stream filtered); session.test.ts: mkdir /tmp/test; skip one interactive-respawn test needing a real PTY (covered by respawn-controller.test.ts). - Full non-mobile suite verified green locally (2680 passed, 12 skipped). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ent tail-file roots - M7 (SSRF): add isSafePushEndpoint (https-only; reject internal/loopback/link-local/metadata IPs incl. IPv4-mapped); enforce in PushSubscribeSchema and re-check before webpush.sendNotification. + unit test. - M1 (command injection): validate tmux session names with isValidMuxName in sessionExists, killSession, and reconcileSessions before they reach a shell call site. - M5: keep the intentional /var/log + ~/logs log-tail roots (a tested feature) and document the wider read scope in docs/security-architecture.md section 5 instead of dropping it. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- engines.node >=18 -> >=22 (Node 18/20 are EOL; CI only tests 22; the start script + systemd unit use NODE_COMPILE_CACHE which needs 22.1+). Updates the README badge and CLAUDE.md requirements to match. - bin: add a 'codeman' alias alongside 'aicodeman' so 'npm i -g aicodeman' provides the 'codeman' command every doc/symlink references (program.name is already 'codeman'; the published package name stays 'aicodeman'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…velope, status codes, /api/v1
Point 1 of the v1.0 lock-in: commit to a stable HTTP API (the cleanest, fullest form).
Core (centralized):
- Every JSON /api response now uses ONE envelope via a Fastify preSerialization hook (src/web/server.ts): success -> { success:true, data:<payload> }; error -> { success:false, error, errorCode } with a conventional HTTP status. Non-JSON routes (file-raw, tail-file SSE, download, screenshots, /q redirect, WS) are skipped.
- Error-code -> HTTP status is a single source of truth (httpStatusForErrorCode in src/types/api.ts): 400/401/404/409/422/429/500. Expanded ApiErrorCode (added UNAUTHORIZED, CONFLICT, RATE_LIMITED). Errors are no longer HTTP 200.
- Versioned alias: /api/v1/* rewrites to /api/* (rewriteApiV1Url), so external clients pin to a stable surface while the bundled UI keeps using /api/*.
- Handlers stripped of manual 'success:true' (50 across 14 route files) so they return bare payloads the hook wraps uniformly; fixed the mux DELETE {success:<bool>} envelope collision (-> {killed}).
Frontend (48 call sites across 10 files):
- _apiJson() auto-unwraps { success:true, data } -> data (null on error), so most bare-shape readers are transparent. Raw-fetch sites relocate payload reads under .data; success/res.ok/error checks unchanged.
Docs: new docs/api-reference.md (envelope, status table, error codes, /api/v1, SSE); versioning-policy.md flipped — the HTTP/SSE API is now part of the stable, SemVer-covered surface.
Verification: full unit/route suite green (2680 passed) incl. ~166 updated assertions across 24 test files; typecheck/lint/format/frontend-syntax clean; a headless-chromium smoke loaded the migrated UI and drove the panels with 0 console/page errors; /api/status and /api/v1/status confirmed returning the uniform envelope live.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>The new CI test gate surfaced a pre-existing flaky timing test: 'should return age of entry' asserted age>=50 after a 50ms setTimeout and measured 49ms on a jittery CI runner. Widened the elapsed-time windows (age >=40/<500; remaining TTL >700/<=960) so they tolerate timer jitter. Pre-existing flakiness, unrelated to the API migration. (File was also normalized by prettier per the pre-commit hook.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ready-unwrapped shared settingsPromise app.js resolves settingsPromise to the unwrapped settings object (env?.data ?? null), matching loadAppSettingsFromServer. The loadQuickStartCases consumer still read settings.data.lastUsedCase, which silently dropped the last-used-case preselection; its fallback fetch also missed the envelope unwrap. Align both with the unwrapped shape. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A 15-agent audit of the merged tree confirmed 9 envelope/contract bugs;
all fixed here, with live-server contract tests added:
Blockers (fresh-install quick start broken):
- session-ui.js runClaude/runShell unwrapped .data from the /api/cases/:name
404 error envelope (which has no data key), so a not-yet-created case threw
TypeError instead of triggering the auto-create fallback. Now '?.data ?? {}'.
Contract violations on the new stable surface:
- Unknown /api routes returned HTTP 404 with {success:true,...} (Fastify's
default not-found payload was wrapped by the envelope hook). Added a
setNotFoundHandler returning the standard error envelope for /api paths.
- POST /api/events/subscribe 400 body became {success:true,data:{error}};
now createErrorResponse(INVALID_INPUT).
- POST /api/clipboard validation error lacked errorCode and shipped HTTP 200;
now createErrorResponse(INVALID_INPUT) -> 400.
- POST /api/run catch path returned bare {success:false,sessionId,error}
(HTTP 200, no errorCode); now OPERATION_FAILED envelope -> 422 with the
dead session id in the message.
- DELETE tail-file/:streamId returned {success: closed}, colliding with the
envelope discriminator; now returns {closed}.
Dead/regressed UI paths:
- Plan history modal could never open: route returned the bare history array
under data while the frontend read data.data.history/currentVersion. Route
now returns {history, currentVersion}; modal task count fixed to stats.total.
- Self-update error toast read j.error.message from the string-typed envelope
error, always falling back to the generic message; now reads the string.
Cleanup:
- Removed the stale QuickStartResponse type (unreferenced; documented the
pre-envelope shape and invited success-key collisions).
Tests: new test/http-contract.test.ts boots a real WebServer (port 3168) and
pins the envelope, /api/v1 alias, error statuses, and the /api 404 shape —
the route-test harness does not install the server-level hook, so these need
the live server. Updated file-routes/plan-routes/scheduled-runs tests to the
fixed shapes.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>Uh oh!
There was an error while loading. Please reload this page.
Ark0N added a commit
to aakhter/Codeman
that referenced
this pull request
Jun 10, 2026
Review follow-ups on PR Ark0N#111 (rebased onto master post-Ark0N#112/Ark0N#113): Resize arbitration redesigned (review blocker 2): the previous 'cols < _ptyCols' guard froze a mobile-only session's PTY at the spawn default — narrow phones rendered clipped and could never re-fit. The guard now uses connection-scoped desktop sizing claims instead: ws-routes registers a claim on a desktop-typed resize and releases it on socket close (or when the same connection later reports a small viewport), and Session.resize() ignores mobile/tablet resizes only while at least one desktop connection holds a claim. A phone alone fully controls its size (shrink, rows-only shrink, re-grow); a phone glancing at a desktop-driven session can no longer reflow it. mobile-handlers' keyboard open/close resize now declares its viewport type so it participates in arbitration. Tests rewritten to cover mobile-only shrink/rows-only/re-grow, claim/release lifecycle, multi- claim behavior, and untyped legacy resizes; ws-routes test covers the claim lifecycle over a real socket. Solo/detached header restored (review blocker 3): index.html had removed #soloSessionTitle and #soloRedockBtn, which _applySoloMode still references — every detached window hit a null deref. Both are back alongside the new mobile utility toggle. Desktop leak fixed (review should-fix): .mobile-header-utility-toggle had no rule outside the <=768px media queries, so the raw button rendered on desktop. styles.css now hides it by default; the mobile/ tablet queries re-enable it. Visual-regression baselines reverted to master (review should-fix): the 18 contributor-machine PNGs are environment-specific (8 of the behavioral tests already report environment-sensitive failures across machines); re-baseline deliberately on the canonical machine instead. The 24 behavioral keyboard/layout/tabs tests are kept as-is. AGENTS.md trimmed to a pointer at CLAUDE.md (review should-fix) to avoid drift between duplicated guidance. Also dropped a dead getAttachmentHistoryForPersist stub (codex-branch residue — no such method exists in src/). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ark0N added a commit
that referenced
this pull request
Jun 10, 2026
- CI section: document the test job (npm run test:ci via vitest.ci.config.ts) and check:frontend-syntax — the "unit suite is excluded" claim was stale - Testing: rewrite rationale (bare npm test fails on browser suites, not tmux) and safety model (TmuxManager in-memory mock under VITEST; the registerTestTmuxSession/snapshot mechanism no longer exists) - API Routes: add the ApiResponse envelope and /api/v1 alias contract - Minor: app.js ~3.6K lines, codeman bin alias, new command-table rows, config-barrel note, generated/gitignored dirs section Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Ark0N pushed a commit
that referenced
this pull request
Jun 10, 2026
Two hardening fixes for the public-tunnel exposure path (COD-54 / COD-55). COD-54 — gate the /api/hook-event localhost bypass when a tunnel is up: `cloudflared --url http://127.0.0.1:port` proxies internet traffic INTO the loopback origin, so a tunneled hook request arrives with req.ip === 127.0.0.1 and the old bare-localhost bypass would pass it unauthenticated. Now: - tunnel running → bypass requires a shared per-instance hook secret (X-Codeman-Hook-Secret header; constant-time compare) + per-IP rate limiting - tunnel not running (loopback-only, the normal case) → unchanged, so already-deployed credential-less hooks keep working. New src/config/hook-secret.ts; auth middleware takes a getTunnelRunning probe (wired from server.ts via tunnelManager.isRunning()). COD-55 — refuse starting the Cloudflare tunnel without auth: enabling the tunnel publishes full terminal control to a public URL; with no CODEMAN_PASSWORD the auth middleware is inactive and the bind guard never trips (tunnel binds loopback). PUT /api/settings now refuses tunnelEnabled:true with a 403 (before persisting) unless CODEMAN_PASSWORD is set or CODEMAN_ALLOW_UNAUTHENTICATED_NETWORK=1 is acknowledged. New isUnauthenticatedNetworkAcknowledged() in network-auth-policy; settings-ui surfaces the refusal as an error toast and reverts the toggle. Scope: the always-on CSRF/Origin guard, Host-header allowlist, and network-auth-policy itself are already upstream (#113) and not re-proposed here. Verification: tsc, eslint, prettier, check:frontend-syntax clean; full test:ci green (2723 passed), incl. test/cod54-hook-event-auth and test/routes/system-routes-tunnel-guard.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pre-1.0 hardening, in three independent commits (docs → CI gate → security). None of this changes runtime feature behavior except the two small security fixes in commit 3.
docs(v1)— addSECURITY.md(private disclosure path + known-limitations) anddocs/versioning-policy.md(defines what 1.0 SemVer covers: CLI + documented env vars are public; the HTTP/SSE API, on-disk state, and experimental features are internal/unstable). FixLICENSE(2024 Claudeman→2024-2026 Codeman) and the staleCLAUDE.mdgotcha claimingxterm-zerolag-inputis hand-duplicated inapp.js— it's single-source now (built into a gitignored vendor bundle bypostinstall.js/build.mjs).test(ci)— actually run the unit suite in CI + close CI blind-spots, and green the pre-existing test debt that surfaced.fix(security)— small input-validation fixes from the 2026-06-09 review (M1/M7) + a doc fix (M5).CI gate (commit 2)
The unit suite was excluded from CI on a now-stale "tests spawn tmux" rationale — but
TmuxManagerno-ops all shell commands underVITEST(test/setup.ts), so it's CI-safe.testjob runsnpm run test:ci(config/vitest.ci.config.ts). Excludes the browser-driventest/mobilesuite (already), plus the other Playwright/chromium tests and timing-flakyperf-*tests — those run separately, not in the fast unit gate.scripts/check-frontend-syntax.mjs,node --checkonsrc/web/public/*.js) wired into the lint job —npm run lintglobs only TS, so a plainSyntaxErrorin a shipped<script>passed CI before; this catches it.text/plainraw body, WS anti-CSWSH) + a backend↔frontend SSE registry parity test.Session not foundasserts (the message now carries the session id), tests for removed helpers (isErroris now internal;createSuccessResponsedeleted), a missingrealpathSyncmock + stale/tmpassertion infile-stream-manager, andsse-subscription-filter(lifecycle events are intentionally broadcast to all clients — only the terminal stream is filtered). Oneinteractive-respawncleanup test isit.skip'd with a TODO (needs a real PTY; incompatible with the VITEST no-op — respawn cleanup is covered by the 165-testrespawn-controllersuite).Security fixes (commit 3)
endpointwasz.string().url()with no host checks, then fetched server-side. AddedisSafePushEndpoint(https-only; rejects internal/loopback/link-local/metadata IPs incl. IPv4-mapped), enforced inPushSubscribeSchemaand re-checked beforesendNotification. + unit test.isValidMuxNamebefore reaching a shell call site insessionExists,killSession, andreconcileSessions(a foreigncodeman-*name with shell metacharacters on the shared socket is now rejected)./var/log+~/logs(a tested feature); documented that wider read scope indocs/security-architecture.md§5 rather than dropping it.Validation
typecheck,lint,format:check,check:lockfile,check:frontend-syntax— all pass.Notes / follow-ups (not in this PR)
playwright install.aicodemanvscodeman), and the Node engines floor (>=18is EOL).🤖 Generated with Claude Code
v1.0 Tier-1 lock-in decisions (added)
Three SemVer-lock-in decisions that are cheap now but breaking after 1.0:
chore(v1)— Node floor>=22(18/20 are EOL; CI tests 22;NODE_COMPILE_CACHEneeds 22.1+) +codemanbin alias (sonpm i -g aicodemanprovides thecodemancommand every doc references;program.namealreadycodeman).feat(api)— stable HTTP contract (cleanest/fullest form). Every JSON/apiresponse is now ONE uniform envelope via a centralpreSerializationhook: success{success:true,data}, error{success:false,error,errorCode}with a conventional 4xx/5xx status (no more 200-on-error). Single error-code→status map; expandedApiErrorCode./api/v1versioned alias (external clients pin it; the bundled UI keeps/api). 50 handlers stripped of manualsuccess:true; 48 frontend call-sites migrated (_apiJsonauto-unwrapsdata); ~166 test assertions updated. Newdocs/api-reference.md;versioning-policy.mdflipped so the HTTP/SSE API is now SemVer-covered.Verification: full unit/route suite green (2680 passed), typecheck/lint/format/frontend-syntax clean, and a headless-chromium smoke loaded the migrated UI and drove the panels with 0 console/page errors.
/api/v1/statusconfirmed returning the uniform envelope.