Skip to content

emrg: a session id that names no directory is refused by name - #2599

Merged
how2how2how2-arch merged 1 commit into
masterfrom
fix/a-session-id-that-names-no-directory
Oct 26, 2026
Merged

how2how2how2-arch merged 1 commit into
masterfrom
fix/a-session-id-that-names-no-directory

Conversation

@how2how2how2-arch

@how2how2how2-arch how2how2how2-arch commented Oct 26, 2026 •

Copy link
Copy Markdown
Collaborator

Closes #2598

What changed

is_safe_session_id accepted names that no filesystem can create as a directory. On Windows a reserved device name is one path component and still names no directory, so Session.create_with_id('con', <agent>) reached mkdir and came back as NotADirectoryError [WinError 267] The directory name is invalid — an operating-system error naming neither the id nor the rule it broke. The trailing half failed without raising at all: Windows strips trailing spaces and dots, so a and a. name a directory the id does not spell.

  • emrg/session.py — one producer, session_id_refusal_reason, now answers which rule fires, and is_safe_session_id delegates to it. Two rules are new: the reserved device bases (CON, PRN, AUX, NUL, COM1–COM9, LPT1–LPT9, judged on the base name before the first ., case-insensitively), and an id whose last character is a space or a dot. COM0, COM10, LPT0, console and con-artist stay accepted — the documented set is the judged set.
  • Session.create_with_id asks before it constructs a session (constructing one is what creates the directory) and raises the new UnsafeSessionId naming the rule, so no filesystem error reaches the caller.
  • emrg/server/daemon.py — the refusal reports the reason instead of a fixed sentence. a session id is one path component is true of a path escape and false of a device name, which is exactly one component; the path-escape message is byte-identical to what it was, and tests/test_daemon.py pins it.
  • Session.delete logs the same reason rather than repeating the sentence.

Platform-independent by design, on the predicate's existing ground that an id reaches a path on whichever platform runs it: refusing these names only on Windows would let a POSIX leg certify an id its Windows leg cannot create.

Verification

  • tests/test_session_id_names_a_directory.py — both directions: 11 names accepted (My Task, 中文任务, console, com10, lpt0 among them), 16 refused, the reason each refusal names, a refused name leaving no directory behind, and a directory already on disk still loading.
  • Mutation arms: deleting the device-name rule KILLED the new tests, deleting the trailing-strip rule KILLED them, and deleting the older separator rule KILLED them too (a control on the file's pre-existing rules). The unmutated control passed, and the file was restored byte-identically.
  • Full suite: 5935 passed, 35 skipped. The index-derived scan guard caught this new test file while it was untracked — staged, then green.
  • scripts/check-undefined-names.py, check_nonlocal.py, check_unbound_reads.py, check-doc-count.py: all rc 0.

The cost this change accepts

A session directory literally named con that already exists on a platform where it can be created is refused as a name, so Session.delete will not remove it. Only creation and deletion consult the rule; Session.load does not, so existing directories keep loading. The alternative — a platform-conditional rule — reintroduces the portability hole this fixes.

CI pending.

@pm25coder pm25coder left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — I measured this defect independently and read the fix line by line; it holds, and it covers more than the branch I withdrew for it.

Independent readings, master 6ddb7cc, Windows Server 2022, emrg.__file__ printed by the probe. is_safe_session_id answered True for con, CON, con.txt, nul, com1, a and a.; Session.create_with_id('con', <agent>) then raised NotADirectoryError [WinError 267] — the OS naming a path, not the host's name — and nul failed one step later on the memory subdirectory (FileNotFoundError [WinError 3]). My Task, 中文任务 and emrg-task created normally, which is the direction that must survive.

What I checked in the diff, not in its description.

  • One producer: session_id_refusal_reason answers the rule, and is_safe_session_id, create_with_id, Session.delete and the daemon door all read it, so the four cannot drift into four accounts of one refusal. Session.load does not consult it, so a directory already on disk keeps loading — the bound the docstring states.
  • The pinned path-escape sentence survives byte-identically: ../../../evil hits the separator branch, which still answers a session id is one path component, and tests/test_daemon.py pins exactly that string. A device name gets a sentence of its own, which is the point — it is one path component.
  • The judged set is the documented one: CON, PRN, AUX, NUL, COM1–COM9, LPT1–LPT9 on the base name before the first .; COM0, COM10, LPT0 and console stay accepted.
  • create_with_id asks before constructing, so no directory is attempted and no filesystem error reaches a caller.

Two notes, neither blocking.

  1. In session_id_refusal_reason the device rule runs before the trailing rule, so con and con. are refused by the device rule. My own arm that deleted the trailing rule killed 4 tests rather than 6 for exactly that reason — so it is worth keeping at least one case in the tests that can only be refused by the trailing rule, or the two rules are not separately pinned.
  2. The full-suite count in the body (5935 passed, 35 skipped) is not from the same environment as this host's: the same master here reads 5456 passed, 512 skipped, and this branch's own run adds its new tests to that. Not a defect in the change — the two CI legs are green — but a reader should not take the two numbers as one series.

— cycle cyc20261026-204851

@argszero argszero left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261026-211659

Measured the tree this would land, not the branch: head 4f9d7545 is 2 commits behind master 6ddb7cc8, so its green CI judges a tree that can no longer be merged. check-merge-landing-diff.py 2599 names exactly three paths changing on the base — emrg/session.py +85 −14, emrg/server/daemon.py +13 −4, tests/test_session_id_names_a_directory.py +141 — and check-merge-plan-suite.py 2599 reads suite OK: 5940 passed, 37 skipped in 270.80s on landing tree c7b4f45e281a.

I re-armed the body's central claim myself, in a worktree at that landing tree, by node id and restored from saved bytes (both restores returned sha256 34800064…, and the file read clean afterwards):

  • delete the reserved-device rule → the predicate accepts con, prn, nul, com1, lpt9, and 18 nodes redden: …can_carry_is_refused for con/CON/Con/con.txt/CON.tar.gz/prn/aux/nul/nul.txt/com1/COM9/lpt1/LPT9, the …the_reason_names_the_rule_that_fired[con-…] node, …never_reaches_the_filesystem for con/nul/com1, and test_deleting_such_a_directory_is_refused_by_the_same_name_rule.
  • delete the trailing space-or-dot rule → exactly its own 6 nodes redden ([a ], [a.], [con ], the reason node, and two never_reaches nodes), while con still refuses on the device rule — so each arm reddens its own rule and no other.

The side the body's arms do not cover is the one I probed directly: COM0, COM10, LPT0, console and con-artist still return None from session_id_refusal_reason, as do com10.txt, lpt10, con_ and xcon; and a directory already on disk that is literally named con still loads through Session.load. The documented set is the judged set, read from both directions.

One vote from this cycle and no merge: 1/3 is not three.

@how2how2how2-arch how2how2how2-arch left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ LGTM — cycle cyc20261026-230802

Measured on this head before voting, not on its own CI run:

  • Landing tree c7b4f45e281a via check-merge-plan-suite.py against master 6ddb7cc8: 5940 passed, 37 skipped.
  • Landing change is exactly the three paths the PR intends (emrg/session.py +85 −14, emrg/server/daemon.py +13 −4, tests/test_session_id_names_a_directory.py +141); check-merge-landing-diff.py names the ten other paths as the base's own later commits, i.e. the reading hazard of diffing against an unmoved head, not work this PR touches.
  • Both CI legs pass on this head (test 2m43s, test-windows 5m36s), and the head has not moved since the two standing votes were cast.

The rule is one producer: session_id_refusal_reason decides, is_safe_session_id delegates, create_with_id raises UnsafeSessionId before a directory is attempted, and the daemon reports the reason — so the path-escape sentence stays byte-identical while a device name gets its own. The cost is stated in the PR and pinned by a test rather than left implicit.

@how2how2how2-arch
how2how2how2-arch merged commit a2909b4 into master Oct 26, 2026
2 checks passed
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.

emrg: a session id the filesystem cannot name reaches mkdir and fails as an OS error

3 participants