Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion .github/workflows/ci-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ name: CI Tests

on:
pull_request:
types: [labeled, synchronize]
# `opened`/`reopened` exist for the require-ci-label guard below: a fresh
# PR without the label gets a visible RED check instead of zero runs.
types: [opened, reopened, labeled, synchronize]

# Opt every JS-based action (checkout, setup-node, cache, pnpm/action-setup,
# setup-python) into the Node 24 runtime ahead of GitHub's 2026-06-02 forced
Expand All @@ -20,6 +22,19 @@ env:
ONNXRUNTIME_NODE_INSTALL: skip

jobs:
# Anti green-by-skip: every other job is gated on the `ci-test` label, so an
# unlabeled PR used to show all-skipped — which satisfies required status
# checks and reads as green. This job INVERTS that: it only runs (and fails)
# when the label is missing, making the untested state visibly red.
require-ci-label:
if: ${{ !contains(github.event.pull_request.labels.*.name, 'ci-test') }}
runs-on: ubuntu-latest
steps:
- name: Fail visibly when the ci-test label is missing
run: |
echo "::error::This PR has no 'ci-test' label — every CI job was SKIPPED (green-by-skip). Add the label to run the suite."
exit 1

test-backend:
if: contains(github.event.pull_request.labels.*.name, 'ci-test')
runs-on: ubuntu-latest
Expand Down
18 changes: 18 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,24 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

_« Socle clean » — the Phase-1 hardening batch from the 2026-07 full audit: security defaults, honest CI, operational reliability, and the two structural taxes (typegen, step dispatch) paid before the continual-learning work starts._

### Fixed (hardening pass — post-merge adversarial review, 7 reviewers over the whole codebase)

- **A panicking DB closure no longer kills the entire backend.** One panic inside a `with_conn` closure poisoned the single shared connection mutex — every later DB call in the process failed with "Mutex poisoned" until restart (observed live). `with_conn` now recovers a poisoned lock (a panicked closure can't leave SQLite mid-transaction) and catches panics at the source, surfacing the panic message in the API error.
- **Scheduled/manual DB backup no longer freezes the whole API.** The SQLite online-copy ran in 5-page steps with a 50ms pause per step *while holding the app's only DB connection* (~2.5s of global freeze per MB — at every boot and every 24h). Now a single-step copy. Also: the boot-tick backup is skipped when a recent backup already exists (a restart loop no longer wipes the retention window), malformed backup env vars warn instead of silently defaulting, and prune failures are logged. Locked by an end-to-end `perform_backup` test.
- **Cron triggers fire exactly once per occurrence.** The stateless "next occurrence within 30s" window overlapped itself: an occurrence landing on the seam fired on BOTH surrounding engine ticks (~1 in 31 → two concurrent runs of the same cron: double worktrees, double PR reviews), and a tick delayed by a slow tracker poll silently *skipped* the occurrence. The engine now evaluates the half-open window `(last-tick, now]`.
- **Cancel means cancelled.** `Cancelled` is now sticky at the SQL level: the runner's late terminal write can no longer flip a user-cancelled run back to `Failed`, the gate-resume window can no longer resurrect it to `Running`, the rollback chain checks the cancel token between compensation steps, and `Pending`/`WaitingApproval` runs (previously uncancellable) can be stopped.
- **A mid-stream Ollama failure now FAILS the step.** The stream-lifetime child exited 0 on every path, so a connection lost mid-generation — or Ollama's in-band `{"error": "model runner has unexpectedly stopped"}` object — produced a *successful* step with truncated or empty output feeding the next steps. The lifeline now reports the stream's real outcome (clean `done` = 0, error/truncation = 1 with the reason in the step error).
- **MCP config writes can never strip secrets again (2026-06-30 incident class).** All six agent-config writers (`.mcp.json`, Codex, Copilot, Gemini, Vibe, Kiro) now ABORT the whole file write when a configured env can't be decrypted — previously they silently wrote `env: {}` (or dropped the entry), clobbering the good secrets already on disk. Same closure applied to the neighbours: Codex `auth.json` is merged (a ChatGPT-OAuth login survives a key sync), a corrupt Gemini `settings.json` is left untouched instead of being replaced, `docs/.kronn.json` isn't rebuilt from default on a read error, the config-migration re-save goes through the atomic writer, and test-mode aborts (with rollback) if its restore state can't be persisted.
- **The LAN-exposure boot guard now checks auth that's actually ENFORCED.** A configured token with `auth_enabled: false` is a middleware no-op — yet it passed the guard (the Docker default: auth off + auto-generated token). The guard now requires enabled auth AND a token; setting `KRONN_AUTH_TOKEN` explicitly now also enables auth (as the guard's own error message always promised); and docker detection uses real container markers (`KRONN_IN_DOCKER` / `/.dockerenv`) instead of `KRONN_DATA_DIR`, which any native user can set (that false "docker" reading disabled the guard and the auth-on default).
- **Interrupted runs finally reach the failure webhook.** `should_notify` included `Interrupted` but the only notify site was the tail of the process that had just *died* — the flagship "cron died at 6am" alert never fired. The boot reconcile now reports what it flipped and the webhook fires at startup; gate-resumed and MCP-triggered runs notify too; `Interrupted` durations no longer poison the `next_check` scheduling average.
- **Engine-side concurrency check is atomic with the run insert** (a manual trigger racing a cron tick could put a `limit=1` workflow at 2 concurrent runs), and a batch step that misses the single `BatchRunFinished` broadcast (channel lag) re-reads the child run from the DB instead of timing out 2h and reporting a false failure.
- **Every unattended network/process path is bounded.** GitHub tracker client (a black-holed connection stalled the ENGINE TICK LOOP — all crons and trackers — until restart), workspace lifecycle hooks (a hung `npm ci` pinned the run and its concurrency slot forever, outside the cancel race), Kiro installer curl, repo-discovery pagination, docs-sidecar POST, P2P WebSocket handshake (a half-open peer permanently blocked reconnection), git push/clone (terminal-prompt + low-speed stall protection), and Ollama chat connect (a firewalled host waited the full 600s before "unreachable").
- **Workflow engine correctness on empty/degenerate agent output:** a `NO_RESULTS` envelope now routes to the action the author declared (the fallback hardcoded `Stop`, hijacking `Goto` recovery branches); a present-but-non-string envelope `status` (`null`, a number) no longer coerces to `"OK"` (failure direction inverted); the Gemini MCP-noise strip is applied even when token count is 0; agent stdout read errors are logged instead of masquerading as EOF.
- **macOS: the "branch already checked out in the main repo" guard actually fires.** git prints canonical paths (`/private/var/…`) while Kronn holds the user's spelling — the equality check missed, and instead of blocking, the code handed the MAIN CHECKOUT back as an isolated worktree. Paths are now compared canonicalized.
- **Silent frontend failures got voices:** a failed workflow launch shows an error toast (was: modal closes, nothing happens, reason in devtools only), launch-variable `pattern`s are validated in the modal (the backend rejection was invisible), a previous launch's late stream callbacks can't write into the next run's live view (generation guard), out-of-order detail fetches can't repaint the wrong workflow, and ~25 silent catch blocks across Settings/MCP/Chat now toast (with optimistic-state revert where no refetch exists).
- **`check-types-drift` really covers unions now.** Tagged unions were compared first-variant-only while the script claimed full coverage — adding a field to `WorkflowTrigger::Tracker` passed CI green. Union of all variants' fields on both sides, quoted-key support, and an honest coverage line (`168 field-compared, 21 skipped…`).
- **DB rows that fail to parse are LOUD:** a workflow whose stored trigger/steps JSON no longer parses logs an error naming it (was: silently became Manual/zero-step and "ran" green), unknown learning statuses warn (a `Rejected` learning silently reappearing as `Pending` rewrote a human decision), editing a concurrently-deleted workflow 404s instead of reporting success.

### Security

- **Secure network defaults — no more accidental LAN exposure.** A stock `docker compose up` now publishes Kronn on `127.0.0.1` only (gateway + frontend), so a fresh install is not reachable from other machines. Exposing to the LAN is an explicit opt-in (`KRONN_BIND=0.0.0.0`) — and when you opt in, the backend **refuses to start unauthenticated**: it requires `KRONN_AUTH_TOKEN`, enabled auth, or an explicit `KRONN_ALLOW_INSECURE_LAN=1` acknowledgment. Closes the "any LAN peer could trigger an Exec step with your credentials (→ host root via the mounted docker socket)" chain flagged by the audit, while keeping the deliberate WSL-backend / Mac-frontend split working (opt-in + token).
Expand Down
3 changes: 3 additions & 0 deletions backend/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ RUN mkdir -p /home/kronn/.local/bin /home/kronn/.local/share/uv \
RUN chmod 1777 /home

ENV KRONN_DATA_DIR=/data
# Explicit container marker — is_docker() must NOT key on KRONN_DATA_DIR (a
# native user can set that too, and a false "docker" fail-opens the LAN guard).
ENV KRONN_IN_DOCKER=1
ENV PATH="/home/kronn/.local/bin:/usr/local/bin:${PATH}"
# The container runs as the non-root `kronn` user, but npm's default global
# prefix (/usr/local) is root-owned → `npm install -g <agent>` fails with
Expand Down
5 changes: 5 additions & 0 deletions backend/scripts/disc-introspection-mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -1940,6 +1940,11 @@ def _current_disc_meta():
try:
url = f"{_backend_url()}/api/discussions/{disc_id}/meta"
req = urllib.request.Request(url, method="GET")
# Same bearer as _http(): without it, an auth-enforced instance 401s
# this read and the silent fallback drops project/agent inheritance.
token = os.environ.get("KRONN_AUTH_TOKEN")
if token:
req.add_header("Authorization", f"Bearer {token}")
with urllib.request.urlopen(req, timeout=5) as resp:
payload = json.loads(resp.read().decode("utf-8"))
data = payload.get("data") or {}
Expand Down
18 changes: 18 additions & 0 deletions backend/scripts/test_disc_introspection_mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -3505,6 +3505,24 @@ def test_http_omits_auth_when_no_token(self):
req = urlopen.call_args.args[0]
self.assertIsNone(req.get_header("Authorization"))

def test_current_disc_meta_adds_bearer_when_token_present(self):
# Regression (Codex audit 2026-07-12): this read used a bare urlopen —
# on an auth-enforced instance it 401'd silently and project/agent
# inheritance fell back to defaults.
cm = mock.MagicMock()
cm.__enter__.return_value.read.return_value = b'{"success": true, "data": {"project_id": "p-1"}}'
cm.__exit__.return_value = False
with mock.patch.dict(os.environ,
{"KRONN_BACKEND_URL": "http://127.0.0.1:3140",
"KRONN_AUTH_TOKEN": "sekret",
"KRONN_DISCUSSION_ID": "d-1"},
clear=True), \
mock.patch("urllib.request.urlopen", return_value=cm) as urlopen:
meta = self.mod._current_disc_meta()
req = urlopen.call_args.args[0]
self.assertEqual(req.get_header("Authorization"), "Bearer sekret")
self.assertEqual(meta.get("project_id"), "p-1")

def test_http_text_adds_bearer_when_token_present(self):
cm = mock.MagicMock()
cm.__enter__.return_value.read.return_value = b'{"kind":"kronn.workflow"}'
Expand Down
Loading
Loading