Skip to content

M1: integridad y seguridad pequeña — 5 propiedades reimplementadas (no portadas) - #27

Merged
DannyBaanks merged 7 commits into
mainfrom
feat/m1-integrity-security
Sep 25, 2026
Merged

DannyBaanks merged 7 commits into
mainfrom
feat/m1-integrity-security

Conversation

@DannyBaanks

Copy link
Copy Markdown
Owner

Qué es esto

Milestone M1 del roadmap (.opencode/plans/munder-upstream-roadmap.md, commit 5fcc3081): cinco
propiedades de integridad y seguridad, reimplementadas en este fork.

Los PRs de chaitanyagiri/munder-difflin se citan como evidencia de una propiedad (qué falla,
qué invariante propone) — nunca como ancestry de código. No hay merge, no hay cherry-pick, no hay
import de sus ramas: los cinco commits son propios, y donde la solución del fork ya era mejor se
conservó la del fork.

Las cinco propiedades

Propiedad Commit Qué estaba mal en el fork
#607 trust de carpeta en Windows bacd885d projects[cwd] se keyeaba con el cwd crudo; Claude lo lee con /
#529 escrituras atómicas 52b5beb7 writeJson escribía in-place y lo usan 10 call sites
#566 ids de mensaje del servidor 5d1c4585 el id lo escribía el agente y es el nombre del archivo del inbox
#564 hold del operador 57944d16 tres caminos automáticos ignoraban el hold
#474 docs generados 07a06528 ensureHive los reescribía en cada mutación

Evidencia Before/After

Cada commit revierte su propia implementación y corre sus tests contra las dos versiones. Ningún
test es decorativo: todos se comprobaron capaces de fallar.

#607   before  8 pass /  2 fail      after 10 pass / 0 fail
#529   before 11 pass /  2 fail      after 13 pass / 0 fail
#566   before  1 pass /  5 fail      after  6 pass / 0 fail
#564   before 33 pass /  4 fail      after 37 pass / 0 fail
#474   before  1 pass /  4 fail      after  5 pass / 0 fail

#607 tiene una divergencia deliberada del upstream: su one-liner normaliza la ruta sin condición, y
en POSIX el backslash es carácter legal de nombre, así que /weird/a\b pre-aceptaría también el
directorio distinto /weird/a/b. Acá se gatea por forma de drive-letter; la variante upstream contra
los mismos tests da 9 pass / 1 fail.

Gates

npm run typecheck exit 0 · npm run check:links ✓ · npm run test:focused 1045 pass / 0 fail /
6 skipped
(1051) · npm run build exit 0 · git diff --check limpio. Verificado en Linux; el CI
de este PR corre typecheck + tests en macOS y build en Linux/Windows/macOS.

Diff

16 archivos, +530/−31: 6 de producción (config.ts, hive.ts, index.ts, workerWake.ts,
queueDelivery.ts, useHive.ts, broadcast.ts) y 10 de test. Ningún cambio ajeno al milestone,
ningún archivo generado, ninguna traducción.

Draft a propósito: el merge es decisión del owner. Este PR existe para correr el CI de tres
sistemas, que en este repo solo se dispara en PR o push a main.

Danny added 5 commits September 25, 2026 15:17
M1 property 1. Upstream evidence: HarnessMD/munder-difflin#607 (cited as
evidence of the property, NOT ported as code).

ensureClaudeProjectTrust keyed projects[cwd] by the raw cwd, so on Windows
the flag landed under 'C:\Users\...' while Claude Code looks it up under
'C:/Users/...' — its parent walk compares with o.startsWith(r + "/") and the
entries it writes on Windows use forward slashes. The folder therefore stays
untrusted, the spawned agent hits the "Accessing workspace" dialog it cannot
answer, that dialog defaults to "No, exit", claude exits 1 and the agent sits
at "waiting". A folder declined once is stranded the same way: the entry was
repaired only under the spelling Claude never reads.

Now every spelling Claude might look up is written, and each is checked on its
own so a trust stored under the raw spelling cannot mask a missing (or
explicitly declined) forward-slash one.

Deliberately NOT the upstream one-liner: that normalises unconditionally, and
backslash is a legal POSIX filename character, so a cwd of /weird/a\b would
also pre-accept the unrelated /weird/a/b. Gated on drive-letter shape instead,
so POSIX keeps exactly one key. UNC is left alone — no evidence of how Claude
keys it.

Tests: 4 cases added to the existing suite in
test/claude-permissions-config.test.cjs (its tmpdir HOME redirect means these
can never touch a real ~/.claude.json).

  before (this commit's src reverted): 8 pass / 2 fail
  after:                            10 pass / 0 fail
  upstream variant, same tests:      9 pass / 1 fail (the POSIX backslash case)

Gates: typecheck exit 0, check:links ok, test:focused 1028 pass / 0 fail /
6 skipped, build exit 0, git diff --check clean.
M1 property 2. Upstream evidence: HarnessMD/munder-difflin#529 (cited as
evidence of the property, NOT ported as code).

Hive.writeJson() wrote in place, and it is the writer ten call sites actually
use: registry.json (3), tasks.json, fleet.json (2), the route cursor and the
hook settings. Between the truncate and the last byte, a force-quit, a
Windows-update reboot or a full disk left invalid JSON behind — and the read
side swallowed it: readJson falls back to whatever default the caller passed,
so the app booted with an empty agent roster and nothing in the log pointing
at why. atomicWriteJson already existed but only three call sites used it.

writeJson now delegates to it, so the guarantee covers all ten at once rather
than depending on each call site remembering to ask. persistConfig() in
config.ts gets the same three steps inline (it does not share Hive's private
helper) — config.json holds the settings, the webhooks and the mission state,
so a truncated one is a silent reset to defaults on the next boot.

A failed write now also removes its temp file: half a temp file is litter in
a directory the inbox/outbox scanners and the ledger guard walk.

Tests: one case per durable file, each simulating the write dying halfway
(half the payload lands, then ENOSPC) through the real node:fs call.

  before (writeJson/persistConfig reverted): 11 pass / 2 fail
  after:                                    13 pass / 0 fail

Gates: typecheck exit 0, check:links ok, test:focused 1030 pass / 0 fail /
6 skipped, build exit 0, git diff --check clean.
M1 property 3. Upstream evidence: HarnessMD/munder-difflin#566 (cited as
evidence of the property, NOT ported as code).

A message id is not a label, it is the inbox FILENAME: deliver() writes
`<id>.json` into the recipient's inbox. An agent-authored id was therefore a
write primitive aimed at the rest of the hive — `../../registry` climbs out of
the inbox, and any name that already exists overwrites a message nobody had
read yet. normalize() now mints the id itself, on both entry points (send()
and the outbox router), so nothing an agent writes can choose it.

PROTOCOL.md already tells agents "the harness fills in `id`", so this
enforces the documented contract rather than adding a new restriction, and
the `<timestamp>-<rand>` shape is kept instead of a bare randomUUID because
ids are compared as strings wherever they stand in for creation order. The
existing `messages()` dedup and the in_reply_to correlation in
realtimeCompletionWatcher both read the id off the stored message, so they
follow the minted value with no change.

The fork had already been bitten by this and worked around it downstream:
useHive.ts abandoned its largest-id high-water mark because one agent's
`dev15-progress-canvas-v4` sorted above every timestamp and froze the wake
loop for the whole floor. The workaround stays; the cause is now closed.

New suite test/hive-message-id.test.cjs covers traversal out of the inbox,
traversal into a sibling agent's folder, two deliveries sharing one authored
id, a guess at a filename already in use, the GPT principal's mailbox, and
the id shape.

  before (normalize honouring partial.id): 1 pass / 5 fail
  after:                                    6 pass / 0 fail

Gates: typecheck exit 0, test:focused 1036 pass / 0 fail / 6 skipped,
messaging suites 35/35, build exit 0, git diff --check clean.
…aths

M1 property 4. Upstream evidence: HarnessMD/munder-difflin#564 (cited as
evidence of the property, NOT ported as code).

`hive.setAgentHold` records the flag and the renderer shows it on the avatar,
but nothing automatic read it: a held agent — one the operator is having a 1:1
with — could still be woken by the main-process beat, still have the
renderer's inbox-wake nudge typed into it, and still receive a `broadcast`.
Three paths, one missing check each.

  - main beat: WorkerWakeFacts gains `onHold`, fed from the registry row the
    loop already has in hand; decide() skips a held worker.
  - renderer: canDeliverToAgent takes the hold and fails closed on it, ahead of
    every other consideration including a manual "send now" — a queued item is
    not lost, it waits for the hold to be released.
  - fan-out: selectBroadcastTargets skips held agents. Direct mail still
    reaches them; the hold silences the floor-wide fan-out, not the human's
    own message to one agent.

Tests: a case per path, each asserting the FALSE flag keeps the ordinary
behaviour, plus a wiring test over useHive.ts source so a fourth delivery gate
added later cannot silently forget the hold (the house pattern for a hook that
cannot load under node:test). compact-latch's source-shape assertion was
widened to accept the new argument instead of being pinned to the old call.

  before (three paths reverted): 33 pass / 4 fail
  after:                        37 pass / 0 fail
  wiring test against HEAD:     fails, naming the call without the flag

Gates: typecheck exit 0, check:links ok, test:focused 1040 pass / 0 fail /
6 skipped, build exit 0, git diff --check clean.
…h on bootstrap

M1 property 5. Upstream evidence: HarnessMD/munder-difflin#474 (cited as
evidence of the property, NOT ported as code).

ensureHive() mixed two responsibilities: making the hive skeleton exist, and
refreshing the two generated docs. But ensureHive is on the runtime path —
ensureAgent and writeTasks both call it — so saving one card or spawning one
agent silently rewrote PROTOCOL.md and COMMANDS.md, the files every agent is
told to treat as the authority. A local edit made on a real floor was reverted
with nothing in the log to say so.

Split in two: ensureHive creates them when absent, and the new
refreshGeneratedDocs() replaces them. The refresh keeps its original purpose —
this repo's own hive once carried a protocol file from the day it was
initialised, so every protocol change since had reached new hives only — by
running where a change can actually land, app start. bootstrapHiveServices()
calls it right after ensureHive, and nothing on the runtime path does.

New suite test/hive-generated-docs.test.cjs: a fresh hive still gets both
files, writeTasks and ensureAgent leave a locally edited copy alone, the
explicit refresh does replace it, and the refresh is wired to app bootstrap.

  before (docs rewritten inside ensureHive): 1 pass / 4 fail
  after:                                     5 pass / 0 fail

Gates: typecheck exit 0, check:links ok, test:focused 1045 pass / 0 fail /
6 skipped, build exit 0, git diff --check clean.
@github-actions

Copy link
Copy Markdown
Contributor

🚫 This PR is missing its before/after evidence

Every pull request here has to show its work. Screenshots or a short screen recording, before the change and after it.

  • Before — no image or video under that heading
  • After — no image or video under that heading

How to fix it: edit the description, keep the ### Before and ### After headings from the template, and drag an image or video under each. GitHub uploads it inline. This check re-runs the moment you save.

A bug fix with no visible surface still needs it: show the failing behaviour, then the same steps passing. A terminal recording is fine.

Genuinely nothing to show — a CI tweak, a typo, a dependency bump? A maintainer can apply the no-visual-change label. Please don't ask unless it truly has no observable effect.

📖 CONTRIBUTING.md → Evidence is mandatory

…rward-slash key

bacd885 writes every spelling Claude Code may look a folder up under; the two
older assertions still expected only the raw C:\ key, so they failed on the
windows-2022 runner while Linux (one key) stayed green.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EUCbpNNP5Kpv7VEKAnuWmt
@DannyBaanks DannyBaanks added the no-visual-change Sin superficie visual: exime el check Before/after evidence label Sep 25, 2026
…sh trust key

Two more tests keyed only the raw C:\ spelling (malformed settings, and the
idempotence check, whose "already complete" fixture must now carry both keys
on Windows). The POSIX-backslash case is skipped on win32: path.join makes it an
ordinary drive path there, where the backslash is the separator.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EUCbpNNP5Kpv7VEKAnuWmt
@DannyBaanks
DannyBaanks marked this pull request as ready for review September 25, 2026 22:13
@DannyBaanks
DannyBaanks merged commit 04642bc into main Sep 25, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-visual-change Sin superficie visual: exime el check Before/after evidence

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant