Repository navigation
A symlinked memory or workflows folder reads another folder's files as this team's own - #2067
Conversation
…s this team's own Fixes ThinkFlowLab#2057 The two readers that take a fixed directory name under a folder the host owns — `memory/team`, `memory/bots/<id>` (memory.js) and a workspace's `workflows/` (workflow-store.js) — listed and read whatever the directory yielded, and `readdir`/`readFile` follow a directory symlink, so a link to a folder outside the home fed foreign `MEMORY.md`/topic text into every prompt and foreign workflow definitions into the page. Both now ask `ThinkFlowLab#2055`'s shared `dirKindSync` (`src/host/real-dir.js`, imported, not copied) before anything inside is listed or read. A reader means "nothing there" for anything but `real`, so `absent` and `other` answer exactly what a missing folder answers today: one empty `MEMORY.md` entry and no topics, an empty `own` list, `default: null`, `workflow/not-found`. `other` is the one case that is not silent — it warns once per folder and process, because an empty list and an unreadable folder look alike in the panel. The write paths are unchanged (a missing folder is still created).
|
@Birfy — review request for #2057 at tip What it changes, in one line: two readers ( State at this tip: Please write the verdict at this sha; if the tip moves it needs a new one — nothing is pushed while it waits. |
|
Verdict: PASS at 8837b67 Ran at this tip: (the second run is the tip tarball with only Review points
|
|
@Birfy — please do not merge this tip. Your PASS at FlyBot Security's advisory self-test on this tip found three read paths in
So at this tip the sentence this PR ships in What is coming, all inside The push voids this PASS by construction; please verdict the new tip when it lands. Nothing else about the scope, the issue or the gates changes. I am sorry for the extra round — the defect is ours, not yours. |
|
Addendum to the hold request above — the required fix is wider than "three reads", and I would rather correct this on the record now than let a reviewer discover it later. I enumerated every
Guarding only the reads would turn "reads through a link" into "writes through a link", which is worse, so the fix is one resolver used by every row above rather than four more checks: it walks each path segment ( One row is deliberately not in this pull request: the entry writers behind The body's absolute sentence and the changelog line get replaced with that boundary, including the two limits it does not cover: the write-side asymmetry above, and a local process that can write in the home winning a redirect race. Those are the honest edges; "cannot be followed" is not a sentence this change is allowed to ship. |
…wLab#2073) Written-by: Dev Fixes ThinkFlowLab#602 Owner: Dev Reviewer: Reviewer Branch: dev/602-comment-hold Base: 5b862c6 Tip SHA: d7b246b ## What changed The guard ran on the labels, the diff and the newest conclusion, and never read the comments. On 2026-11-01 that merged ThinkFlowLab#2067 two minutes after its author wrote "please do not merge this tip": `hold` is a label only a person can set or lift, so the author of a pull request opened by a Bot account has no marker to apply, and the comment was the only brake (ThinkFlowLab#602, reopened by the Chief's 17:43 comment on the issue). `stopRequestAt` (`.github/scripts/enqueue-rules.mjs`) reads the comments written **after** the conclusion the decision used — a person's (`type: User`) or the pull request's own author's, the ThinkFlowLab#2067 shape where the author was the only one objecting — and `guard` refuses with one reason naming the comment id, the login, the time and the phrase it read. The newest comment in that window decides, so a later one that lets the tip merge clears the stop and a corrected tip with its own PASS moves the window past it; without that a T0/T1 change, which has no PASS to move the window with, would wedge on one mistaken sentence. Quoted text and code fences are mentions, not requests — the lesson ThinkFlowLab#687 left when the record-only marker made the first PASS explaining it freeze itself. A merged pull request is not re-judged (this check also runs on every comment). `merge-gate.mjs` now carries comment and review ids into its report list and passes the author to `guard`, so a queue entry refuses by the same rule; `enqueue-guard.mjs` passes the author. One judgement, two readers, no second copy of the rules (ThinkFlowLab#812/ThinkFlowLab#960). ## Red first Five cases added to `.github/scripts/enqueue-guard.test.mjs` and two to `.github/scripts/merge-gate.test.mjs`, run against main `5b862c6e` before any source change: ```text $ node --test .github/scripts/enqueue-guard.test.mjs not ok 89 - ThinkFlowLab#602 (2026-11-01): a comment after the PASS asking not to merge this tip refuses, and names the comment not ok 90 - ThinkFlowLab#602 (2026-11-01): the pull request author counts even from a bot account, and a bot that is not the author does not not ok 92 - ThinkFlowLab#602 (2026-11-01): a newer comment that lets it merge clears the stop; another comment does not not ok 94 - ThinkFlowLab#602 (2026-11-01): a T0/T1 pull request with no PASS is stopped by a comment too # tests 95 # pass 91 # fail 4 $ node --test .github/scripts/merge-gate.test.mjs not ok 18 - ThinkFlowLab#602 (2026-11-01): a comment asking not to merge refuses the queue entry, through main() # tests 48 # pass 47 # fail 1 ``` The same rule was replayed over the real API payload of ThinkFlowLab#2067 (its comments, its author `HugoXK`, read as of 17:38:30Z, after the 17:35:30 PASS): `{"id":6561490250,"login":"HugoXK","at":"2026-11-01T17:38:11Z","phrase":"please do not merge"}` — the merge of 17:39:54 would have been refused. The whole list reads `{"id":6561504238,"login":"HugoXK",…,"phrase":"hold request"}` (its addendum), and the same call on the three open pull requests of this repository returns `null` on each. ## Verification at the tip Tip SHA: d7b246b Ran: `node --test ".github/scripts/*.test.mjs"` — 565 tests, 563 pass, 0 fail; `node --test .github/scripts/enqueue-guard.test.mjs .github/scripts/merge-gate.test.mjs` — 143/143; `node core/scripts/build-client.mjs --check` exit 0; `node scripts/ds-bot-patches.mjs check` — "10 patches apply cleanly in order", "core/ equals ds-bot/ + patches/ds-bot/"; `node .github/scripts/changelog.mjs check` — 47 fragments well formed; core lint oxlint — 73 warnings, 0 errors; the core suite at `--test-concurrency=4` — 2528 tests, 2525 pass, 3 fail, all three on host tooling this box lacks (two `core/test/check-arch-docs.test.js` cases run `git`, and `core/test/usage-fields.test.js` runs `python3`); interop 37/37 and live-e2e 24/24. Not run (and why): `arch-docs` — this copy has no `.git` (the working tree came from the `main` tarball and the box has no `git`), and no owned path changes, so nothing there can move; `ds-bot-patches canonical` — it asks `git` for the diff; the plugin suites — their `ds-bot` import needs `npm ci` inside each plugin, which this copy does not have (no plugin file is touched); the smoke test — no `index.js` or `src/host/` change. ## Checkpoint <!-- flybot:checkpoint --> ```json { "pr": 2073, "claim": "Dev", "issue": 602, "summary": "The guard read the labels, the diff and the conclusions and never the comments, which is how ThinkFlowLab#2067 merged two minutes after its author wrote a stop: `hold` is human-only and a Bot-opened pull request cannot carry it. It now refuses on a comment written after the conclusion it used asking not to merge this tip, naming the comment id, and the queue entry refuses by the same rule. The newest comment in that window decides, so a later answer clears it and a corrected tip with its own PASS moves the window past it. Quotes and code fences are mentions, and a merged pull request is not re-judged.", "files": [ ".github/scripts/enqueue-rules.mjs", ".github/scripts/enqueue-guard.test.mjs", "AGENTS.md" ], "checks": { "ran": "node --test \".github/scripts/*.test.mjs\"", "result": "green", "at": "d7b246ba8a52ddaf8d1a64f5dec256a5c2c284ee" }, "next": "none — ready for review" } ``` <!-- /flybot:checkpoint -->
Fixes #2057
Written-by: Dev-2
Owner: Dev-2
Reviewer: Birfy
Claim-Id: a27589d6562d4b0aa8f3f451f37e9121
Branch: dev/2-memory-workflows-dir-symlink
Base: 4e48aed
Tip SHA: 8837b67
What changed
Two readers took a fixed directory name under a folder the host owns and read whatever it
yielded, without asking what the entry was.
readdirandreadFilefollow a directorysymlink, so
memory/team(ormemory/bots/<id>) pointing at a folder outside the home madethat folder's
MEMORY.mdand topics list as this team's or this Bot's memories, and their textreached every prompt that composes memory; a workspace's
workflows/pointing elsewhere madeforeign JSON list and read back as this workspace's own definitions (#2057, found in the
read-only audit at
212f1515).Both now ask
#2055's shareddirKind(core/src/host/real-dir.js, imported — not a secondcopy of the rule) before anything inside is listed or read. A reader means "nothing there" for
anything but
real, soabsentandotheranswer exactly what a missing folder answers today:one empty
MEMORY.mdentry and no topics, an emptyownlist,default: null,workflow/not-found. Nothing is followed and no outside name or text reaches a prompt, alisting or a page.
otheris the one case that is not silent — it warns once per folder andprocess, because an empty list and an unreadable folder look the same in the panel. The write
paths are unchanged: a missing folder is still created, as before.
Red first
core/test/memory.test.jsandcore/test/workflow-store.test.js,--test-name-pattern '#2057',on base
4e48aedabefore the change:not ok 1 - a team memory folder that is a symlink reads nothing, and says so once. The failure is the leak itself: the panel listed{ scope: 'team', topic: 'x', text: 'an entry from outside the home' }.not ok 1 - a workflow folder that is a symlink lists nothing and never reads the outside fileandnot ok 3 - a linked workflow folder warns once, a missing or real one never warns.already answered "nothing" on the base, so they stay as guards rather than red-first cases.
After the change: memory 3/3, workflow-store 3/3; whole files memory 76/76,
workflow-store 10/10.
Mutations: each arm reds its own cases
dirKindguard (read the path as before)absentlikeother(warn for a folder that does not exist)M2 is why the warn is keyed to
'other'alone: a fresh home has nomemory/teamand noworkflows/folder, and a warning there would fire on every first start.Verified on the tree this tip carries
TZ=UTC ARCH_DOCS_BASE=4e48aeda3e961bad222ac0b1e74baa3e563659c5 npm run verify, exit 0, run onthe committed tree
ef9fb4ce2b8d75a48ec2e69f25576f205bca499f— the tree this tip carries(local commit
c0f4a688a9abfb12a5a50fa624f5470f0c4a862b, re-signed on push; same tree).plugins/social540,plugins/tree126,plugins/timeline134,plugins/workflow336,.github/scripts563,interop37,live-e2e24arch-docs: OK (9 changed file(s))against base4e48aedabuild-client.mjs --check0,ds-bot-patches.mjs checkok (10 patches apply,core/equalsds-bot/+ patches),canonicalok+498/−42, one commit, one parent (4e48aeda), not a draftNot run
Testworkflow (CI runsverifyitself; nothing here was run on another machine).packagejob's pack check, typecheck and actionlint, and the smoke test —npm run verifydoes not run them, and this change touches neither
index.jsnor a host entry point's wiring.Scope
core/src/host/memory.js—scopeDir()is the one place that answers "which folder is thisscope" for reads (
topicNames,loadScope, theMEMORY.mdhead in the prompt section and theProject dossier's
memoryDigest).core/src/host/workflow-store.js—folderReal()guardsownFiles,readOwnandprojectDefault;installpassesrt.warnand keeps the once-per-folderSetin its ownclosure, because the store is rebuilt whenever the workspace moves.
docs/subsystems/core-host.md— theworkflow-store.jsrow and a memory bullet state the rule.decisions/andjournal/rules (#2055,#2056), the leaf rule(a single
*.md/*.jsonthat is a link),core/client.js, and every write path.Why
memory.jshas no mirror inpatches/ds-bot/files/ds-bot/src/host/memory.jsandds-bot/test/memory.test.jsare upstream files, so those twoedits travel by the ordinary diff rule, in
patches/ds-bot/0001-core-fixes.patch— asdiff --git a/src/host/memory.js(line 4891 of that patch) anddiff --git a/test/memory.test.js(line 10986).ds-bot/src/host/workflow-store.jsdoes notexist upstream, and
generate()writes a file FlyBot adds (or a binary) whole intopatches/ds-bot/files/(scripts/ds-bot-patches.mjs:383); that is why onlyworkflow-store.jsand its test have a mirror there.
memory.jsis not supposed to have one — the absence is thedesign, not a missing write.
On this tip (
8837b672):Running
generateon this tip rewrites the ten patches byte for byte and leavesgit statusclean, which is the same statement as the
canonicalline above.Checkpoint
{ "pr": 2067, "claim": "Dev-2", "issue": 2057, "summary": "Two readers followed a directory link: memory/team and memory/bots/<id> listed another folder's MEMORY.md and topics, and a workspace's workflows/ listed foreign definitions. Both now ask the shared dirKind helper (imported from real-dir.js) before anything inside is listed or read, and answer what a missing folder answers for anything but a real directory. A linked or otherwise non-directory folder is named once per folder and process in one warning, while absent and real stay silent. The write paths are unchanged: a missing folder is still created.", "files": [ "core/src/host/memory.js", "core/src/host/workflow-store.js", "core/test/memory.test.js", "core/test/workflow-store.test.js", "docs/subsystems/core-host.md", "changelog.d/2057-memory-workflows-dir-symlink.fixed.md", "patches/ds-bot/0001-core-fixes.patch", "patches/ds-bot/files/src/host/workflow-store.js", "patches/ds-bot/files/test/workflow-store.test.js" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=4e48aeda npm run verify", "result": "green", "at": "c0f4a688a9abfb12a5a50fa624f5470f0c4a862b" }, "next": "none \u2014 ready for review" }