Feat/tty fwd#3
Conversation
Run a shell in a local PTY and attach it to dbclient netcat mode via stdin/stdout, so devices can expose a terminal to a remote TCP endpoint without listening on any local port. Co-authored-by: Cursor <cursoragent@cursor.com>
Give forwarded shells a fixed winsize and TERM so full-screen programs work over the bridge, and add a small WebSocket/xterm.js demo to test tty-fwd end-to-end over SSH -B. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces tty-fwd, a cloud terminal bridge that runs a local shell in a pseudo-terminal and forwards its I/O to a remote TCP endpoint over SSH using dbclient netcat mode. It also includes a Node.js-based demo server to bridge the terminal stream to a browser via WebSockets. The feedback focuses on improving the robustness of tty-fwd.c, including fixing potential false positives in argument parsing, preventing permission failures for non-root users by removing pty_setowner, using execlp to support non-absolute shell paths, adopting Dropbear's portable mysignal wrapper, and safely handling file descriptor closures to avoid accidentally closing standard streams.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| static int argv_has_netcat_opt(int argc, char **argv) { | ||
| int i, j; | ||
|
|
||
| for (i = 1; i < argc; i++) { | ||
| if (argv[i][0] != '-') { | ||
| continue; | ||
| } | ||
| for (j = 1; argv[i][j]; j++) { | ||
| if (argv[i][j] != 'B') { | ||
| continue; | ||
| } | ||
| if (argv[i][j + 1] != '\0') { | ||
| return 1; | ||
| } | ||
| if (i + 1 < argc) { | ||
| return 1; | ||
| } | ||
| return 0; | ||
| } | ||
| } | ||
| return 0; | ||
| } |
There was a problem hiding this comment.
The current implementation of argv_has_netcat_opt can produce false positives if an option argument or a long option starts with - and contains the character B (for example, -bBind or --hostB). To prevent this, we should ensure that B is only recognized as a valid option character (i.e., not part of an option argument) by skipping double-dash options and verifying that any preceding characters in the option group are known flags that do not take arguments.
static int argv_has_netcat_opt(int argc, char **argv) {
int i, j;
for (i = 1; i < argc; i++) {
if (argv[i][0] != '-' || argv[i][1] == '-') {
continue;
}
for (j = 1; argv[i][j]; j++) {
if (argv[i][j] == 'B') {
/* To avoid matching arguments of other options (e.g. -bBind),
* 'B' must only be preceded by options that do not take arguments. */
int k;
int ok = 1;
for (k = 1; k < j; k++) {
char c = argv[i][k];
/* List of common dbclient flags that do not take arguments */
if (c != 'y' && c != 'v' && c != 't' && c != 'T' && c != 'N' && c != 'f' && c != 'q') {
ok = 0;
break;
}
}
if (ok) {
if (argv[i][j + 1] != '\\0' || i + 1 < argc) {
return 1;
}
}
}
}
}
return 0;
}| pw = getpwuid(getuid()); | ||
| if (pw) { | ||
| pty_setowner(pw, (char*)tty_name); | ||
| } |
There was a problem hiding this comment.
Calling pty_setowner is unnecessary for tty-fwd because the tool is run by the local user, meaning the allocated PTY is already owned by them. Furthermore, pty_setowner attempts to chown the PTY to the tty group, which will fail with EPERM and cause tty-fwd to exit immediately if run as a non-root user. Removing this call ensures the tool can be used by non-root users.
/* pty_setowner is not needed for local non-root execution */| execl(shell, shell, (char*)NULL); | ||
| _exit(EXIT_FAILURE); |
There was a problem hiding this comment.
Using execl requires the shell argument to be an absolute path. If a user passes a simple shell name (e.g., --shell sh), execl will fail to find it. Using execlp instead will search the PATH environment variable when the shell name does not contain a slash, making the tool much more robust and user-friendly.
execlp(shell, shell, (char*)NULL);
_exit(EXIT_FAILURE);| if (signal(SIGINT, shell_signal_handler) == SIG_ERR | ||
| || signal(SIGTERM, shell_signal_handler) == SIG_ERR) { | ||
| dropbear_exit("signal() error"); | ||
| } |
There was a problem hiding this comment.
Using the standard signal function can lead to unreliable signal semantics on some platforms (e.g., resetting the handler to SIG_DFL or not blocking the signal during execution). Dropbear provides a portable wrapper mysignal which should be used instead to ensure consistent and reliable signal handling.
if (mysignal(SIGINT, shell_signal_handler) == SIG_ERR
|| mysignal(SIGTERM, shell_signal_handler) == SIG_ERR) {
dropbear_exit("signal() error");
}| if (dup2(master, STDIN_FILENO) < 0 || dup2(master, STDOUT_FILENO) < 0) { | ||
| dropbear_exit("dup2 failed: %s", strerror(errno)); | ||
| } | ||
| close(master); |
There was a problem hiding this comment.
If master happens to be STDIN_FILENO or STDOUT_FILENO (which can occur if the process was started with closed standard streams), calling close(master) after dup2 will close the standard stream we just duplicated to. To prevent this, only close master if it is greater than STDOUT_FILENO.
if (dup2(master, STDIN_FILENO) < 0 || dup2(master, STDOUT_FILENO) < 0) {
dropbear_exit("dup2 failed: %s", strerror(errno));
}
if (master > STDOUT_FILENO) {
close(master);
}| if (dup2(slave, STDIN_FILENO) < 0 | ||
| || dup2(slave, STDOUT_FILENO) < 0 | ||
| || dup2(slave, STDERR_FILENO) < 0) { | ||
| _exit(EXIT_FAILURE); | ||
| } | ||
| close(slave); |
There was a problem hiding this comment.
Similarly to the master fd, if slave happens to be one of the standard streams (0, 1, or 2), calling close(slave) after dup2 will close the stream we just duplicated to. Only close slave if it is greater than STDERR_FILENO.
if (dup2(slave, STDIN_FILENO) < 0
|| dup2(slave, STDOUT_FILENO) < 0
|| dup2(slave, STDERR_FILENO) < 0) {
_exit(EXIT_FAILURE);
}
if (slave > STDERR_FILENO) {
close(slave);
}1 participant
No description provided.