Skip to content

Merge current upstream and make E2E cleanup ownership-safe - #1

Open
xz-dev wants to merge 9 commits into
bmaurer:backpressure-flow-controlfrom
xz-dev:pr730-followup
Open

Merge current upstream and make E2E cleanup ownership-safe#1
xz-dev wants to merge 9 commits into
bmaurer:backpressure-flow-controlfrom
xz-dev:pr730-followup

Conversation

@xz-dev

@xz-dev xz-dev commented Aug 27, 2026

Copy link
Copy Markdown

Summary

This is a focused follow-up to your backpressure-flow-control branch and upstream MisterTea#730.

  • merge current MisterTea/EternalTerminal:master (b74a12ef) into the existing branch
  • resolve the single TerminalClient::run conflict by preserving both the flow-control output drain and upstream's non-TTY consoleFd >= 0 guard from Keep the client alive when the console fd is not a tty MisterTea/EternalTerminal#788
  • make the existing E2E harness ownership-safe: fixed-port conflicts now fail closed, and cleanup stops only child processes started by that invocation
  • keep your existing three commits and authorship unchanged; no new test harness or feature scope is added

Why the harness cleanup change is included

The existing cleanup used kill -9 $(lsof -t -i:...) and broad pkill -f, which could terminate an unrelated test or service using ports 4444/4445/4446. The runners now refuse occupied ports instead of reclaiming them, use per-invocation FIFO/socket/log paths, and track only their own child PIDs. run_all_scenarios.sh no longer performs broad pre-scenario cleanup or restores the whole worktree.

Validation

  • CMake configure/build with DISABLE_VCPKG=ON, BUILD_TESTING=ON, RelWithDebInfo: passed
  • ctest --test-dir build-followup --output-on-failure -j2: 159/159 passed; 2 privilege-dependent skips
  • clang-format 18.1.8 dry-run on changed C++ files: passed
  • git diff --check: passed
  • bash -n on all changed shell runners: passed
  • occupied-port safety checks for run_e2e_test.sh, do_scenario.sh, and throughput_test.sh: each refused to start and left the task-owned listener alive

This PR targets Ben's branch directly so the original upstream PR remains the primary contribution.

MisterTea and others added 8 commits July 12, 2026 18:03
Co-authored-by: Cursor <cursoragent@cursor.com>
* Fix GCC 15 build failure for Ubuntu 26.04 (Resolute)

- Conditionally link stdc++fs only when the library exists (removed in GCC 15)
- Add explicit #include <cstdint> to source files using fixed-width integer types
- Fixes PPA build failure: https://launchpad.net/~jgmath2000/+archive/ubuntu/et/+build/33370536
- Related: MisterTea#744, MisterTea#581

* style: order cstdint includes for clang-format

* style: apply clang-format to cstdint includes
* Fix GCC 16 CI dependency compatibility

* Add autoreconf CI dependencies

* Fix SimpleIni header discovery

* Fix macOS build portability

* work

* more compat fixes

* fixes

* fix cleanup

* Fix FreeBSD portability workflow

Co-authored-by: Cursor <cursoragent@cursor.com>

* work

* fix

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Jason Gauci <jgmath2000@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
* add windows deployment workflows

* Trigger Windows asset deployment on release publish

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Jason Gauci <jgmath2000@gmail.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
…es. (MisterTea#784)

* Fix four pre-auth and privilege-escalation issues from security notices.

Address ANT-2026-5PETM5BV, ANT-2026-VAMER5RC, ANT-2026-AVTT7HQH, and
ANT-2026-A3WQS3AG without bumping PROTOCOL_VERSION.

ANT-2026-5PETM5BV (pre-auth DoS): The fixed 8-thread client handler pool
read ConnectRequest via readProto, which trusted a client length up to
128 MiB and allocated up front. readAll() also reset its 30s idle timer
on every byte, so a slow trickle could hold a worker forever. Cap
handshake protos at 4 KiB, and enforce both idle and absolute (60s)
deadlines in readAll—checking the absolute deadline every loop iteration
so a steady trickle cannot bypass it.

ANT-2026-VAMER5RC (pre-auth reconnect crash / disconnect): Returning
clients were accepted on cleartext clientId alone. recoverClient() closed
the live victim socket before reading a plaintext SequenceHeader, and
BackedWriter::recover() used STFATAL/LOG(FATAL) when the attacker-supplied
sequence was ahead of the server—aborting the root daemon. Treat a bad
sequence as a caught runtime_error, and only close the old socket after
recover succeeds so a failed reconnect leaves the session intact.

ANT-2026-AVTT7HQH / ANT-2026-A3WQS3AG (unix-socket LPE as root): etserver
never drops privileges, so reverse-tunnel sources ran root unlink/bind/
chmod/chown on client-chosen paths (arbitrary file delete + chown TOCTOU),
and forward destinations connected as root to arbitrary AF_UNIX paths
(e.g. docker.sock). Add UserSocketOps: fork, setgroups/setgid/setuid to
the session user, perform listen/connect, and return the fd via SCM_RIGHTS.
PortForwardHandler uses this for unix source listen and destination
connect; path-based chown after bind is removed.

Unsolved without a PROTOCOL_VERSION bump: reconnect still does not prove
passkey knowledge before recover. ConnectRequest carries only clientId and
version; adding challenge-response (or encrypting the recover handshake)
would break old clients that must match PROTOCOL_VERSION exactly. Residual
risk: an on-path observer who sniffs a live clientId and supplies an
acceptable sequence number can still displace that session's TCP
connection and force-disconnect the victim. They cannot speak the
encrypted session without the passkey, and they can no longer crash the
daemon with a crafted sequence. Full reconnect authentication remains a
coordinated future protocol change.

Add SecurityNoticesTest and UserSocketOps coverage for each fixed case.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Fix security notice tests when CI runs as root.

Debian/FreeBSD portability jobs run et-test as root, so DAC-based checks
(chmod 0555 directories, mode-000 sockets) were bypassed when the session uid
was also 0. Target an unprivileged user (nobody) for privilege-drop
tests so setuid actually loses the ability to unlink/connect, matching
the etserver-as-root threat model.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Harden absolute-timeout security test against CI hangs.

Make the trickle socket non-blocking and always close/join the writer via
RAII so a failed assertion cannot leave a stuck thread. Also reject macOS's
nobody sentinel uid when selecting a privilege-drop test user.

Co-authored-by: Cursor <cursoragent@cursor.com>

* Raise patch coverage for UserSocketOps and privilege-dropped sockets.

Expose in-process listen/connect helpers, flush gcov before forked _exit,
and add tests for error paths, PipeSocketHandler as-user APIs, and
recoverClient success.

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Jason Gauci <jason.gauci@webai.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ddf67a5 (MisterTea#739) replaced FATAL_FAIL(rc) in TerminalClient::run()'s console
read loop with explicit handling of every read() outcome. That correctly
fixed the spurious ENOENT crash, but it also turned rc == 0 into an
unconditional "clean session end":

    } else if (rc == 0) {
      LOG(INFO) << "Console EOF";
      break;
    }

That treats EOF as "the user closed an interactive session", which only
holds when the console descriptor is actually a terminal. Often it is not.

Console::getFd() returns STDOUT_FILENO, so the loop reads *stdout*, not
stdin. At a normal terminal, fds 0, 1 and 2 all refer to the same tty, so
reading fd 1 works and the distinction never shows. Under nohup(1) it does:
nohup redirects stdout to nohup.out while leaving the tty on stdin, so the
client ends up reading a regular file. select() always reports a regular
file ready, and the read always returns 0 -- the file is empty at startup,
and the shared offset then tracks the end of the file as the client writes
terminal output to it. The client therefore exits moments after connecting,
before the session can be used:

    [INFO] Got command: echo hello
    [INFO] Console EOF
    [INFO] Shutting down connection

This breaks any workflow that backgrounds the client so it outlives the
process that launched it: nohup et host &, service managers, and IDE
remote-development integrations that hold port forwards open across editor
restarts. A plain redirect is affected too -- et host --command '...' >
out.txt opens stdout O_WRONLY, so read() returns EBADF and takes the other
break added by MisterTea#739.

Before MisterTea#739 this was invisible: FATAL_FAIL(X) fires only on X == -1, so
rc == 0 fell through silently and the session survived, busy-looping on a
descriptor it could never read from.

Treat EOF and hard read errors as session-fatal only when the console
descriptor is a tty. Otherwise stop selecting on it and keep running.
Console input is impossible either way -- it always was under nohup -- but
the session and its port forwards stay up. Terminal output is unaffected:
it still goes to the same descriptor.

This completes MisterTea#739 rather than reverting any of it. Every read() outcome
is still handled explicitly and the errno-corruption crash stays fixed; a
non-interactive console simply no longer ends the session.

Testing:

Adds NonTtyConsoleKeepsSessionAlive to test/integration_tests/TerminalTest.cpp.
It drives a real TerminalClient with a FileBackedConsole whose descriptor is a
regular file opened O_RDWR|O_APPEND -- the shape nohup installs -- and requires
that run() has not returned once the connection is up, and that remote output
still reaches that descriptor. Pre-fix, run() returns within milliseconds.

Verified manually on macOS: with a patched client, a remote session launched
under nohup stays up and its port forwards carry traffic, where an unpatched
build exited immediately after login.

Co-authored-by: jwshort <jwshort@devvm14159.vll0.facebook.com>
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.

6 participants