feat: implement TELNET protocol support with authentication and comma…#253
Merged
mariocandela merged 2 commits intomainfrom Jan 24, 2026
Merged
feat: implement TELNET protocol support with authentication and comma…#253mariocandela merged 2 commits intomainfrom
mariocandela merged 2 commits intomainfrom
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds first-class Telnet protocol support: new TELNET strategy implementation, configs and integration tests, builder wiring, tracer instrumentation for Telnet events, and CI coverage exclusion for the new file. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Telnet Client
participant Server as TelnetStrategy
participant Tracer as Tracer
participant History as HistoryStore
participant LLM as LLM Handler
Client->>Server: TCP connect
Server->>Server: Perform IAC negotiation
Server->>Client: Send login prompt
Client->>Server: Send username/password
Server->>Server: Validate password (regex)
Server->>Tracer: Emit login event
Server->>History: Create session record
loop interactive commands
Client->>Server: Send command line
Server->>Server: Match command (regex)
alt matched and LLM-eligible
Server->>LLM: Invoke model for response
LLM-->>Server: Return generated output
Server->>Client: Send output
else matched (static handler)
Server->>Client: Send configured response
else not matched
Server->>Client: Send "command not found"
end
Server->>History: Append interaction
Server->>Tracer: Emit interaction event
end
Client->>Server: Disconnect
Server->>Tracer: Emit session end event
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #253 +/- ##
==========================================
+ Coverage 81.55% 81.76% +0.20%
==========================================
Files 7 7
Lines 618 625 +7
==========================================
+ Hits 504 511 +7
Misses 93 93
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@integration_test/integration_test.go`:
- Around line 45-68: Update the misleading comment on readTelnetUntil to remove
the "skipping IAC sequences" claim (or implement IAC filtering if intended), and
change the overflow behavior so that when fullResponse grows beyond 8192 bytes
the function returns an error instead of (fullResponse, nil); use a descriptive
error (e.g., via fmt.Errorf) that callers can assert on. Ensure you add the
"fmt" import if you create the error and keep the rest of readTelnetUntil (conn
*net.TCPConn, expected string) behavior the same.
In `@protocols/strategies/TELNET/telnet.go`:
- Around line 305-341: In readLine, rename the local identifier currently named
`byte` to e.g. `ch` to avoid shadowing the builtin, add proper error handling
for the two conn.Read calls used to skip IAC bytes (check and return the error
if any read fails), and replace the ad-hoc `i`/256 check with a proper
max-line-length guard (define a constant like maxLineLen and return an error if
line length exceeds it) so the `line` slice cannot grow unbounded; ensure all
conn.Read results are checked and propagate errors from read operations in the
readLine function.
- Around line 287-303: The function drainInitialInput currently clears the
connection deadline by calling conn.SetReadDeadline(time.Time{}), which disables
the configured session timeout; change drainInitialInput to preserve and restore
the session deadline (either by reading the current deadline before setting the
short 200ms read deadline and restoring it after draining, or by accepting the
configured deadlineSeconds and calling conn.SetDeadline(time.Now().Add(...))
when finished) so the configured deadlineTimeoutSeconds remains enforced for the
remainder of the session; update references to drainInitialInput accordingly.
🧹 Nitpick comments (4)
protocols/strategies/TELNET/telnet.go (3)
135-140: Consider not logging thePasswordRegexpattern in error messages.The static analysis flagged this line. While logging the regex pattern (not actual passwords) is lower risk, it could reveal which passwords the honeypot accepts to anyone with log access. Consider logging a generic error without the pattern:
🛠️ Suggested fix
matched, err := regexp.MatchString(servConf.PasswordRegex, password) if err != nil { - log.Errorf("error regex: %s, %s", servConf.PasswordRegex, err.Error()) + log.Errorf("error matching password regex: %s", err.Error()) conn.Write([]byte("Login incorrect\r\n")) return }
278-285:negotiateTelnetignores write errors.If the writes fail, the function silently continues. Consider logging or returning errors to aid debugging connection issues.
🛠️ Suggested fix
-func negotiateTelnet(conn net.Conn) { +func negotiateTelnet(conn net.Conn) error { // Minimal telnet negotiation - conn.Write([]byte{IAC, WILL, ECHO}) - conn.Write([]byte{IAC, WILL, SUPPRESS_GO_AHEAD}) + if _, err := conn.Write([]byte{IAC, WILL, ECHO}); err != nil { + return err + } + if _, err := conn.Write([]byte{IAC, WILL, SUPPRESS_GO_AHEAD}); err != nil { + return err + } + return nil }
343-357: Remove the unusedstripIACfunction.This function is defined but never called anywhere in the codebase and should be removed as dead code.
integration_test/integration_test.go (1)
295-577: Consider extracting a helper for Telnet authentication.The authentication sequence (connect → login → password → prompt) is repeated across all six Telnet tests. Extracting this into a helper would reduce duplication and make tests more maintainable.
Example helper
// authenticateTelnet connects and authenticates, returning the connection ready for commands func (suite *IntegrationTestSuite) authenticateTelnet(user, pass string) (*net.TCPConn, error) { tcpAddr, err := net.ResolveTCPAddr("tcp", suite.telnetHoneypotHost) if err != nil { return nil, err } conn, err := net.DialTCP("tcp", nil, tcpAddr) if err != nil { return nil, err } conn.SetDeadline(time.Now().Add(5 * time.Second)) if _, err := readTelnetUntil(conn, "login:"); err != nil { conn.Close() return nil, err } if _, err := conn.Write([]byte(user + "\r\n")); err != nil { conn.Close() return nil, err } if _, err := readTelnetUntil(conn, "Password:"); err != nil { conn.Close() return nil, err } if _, err := conn.Write([]byte(pass + "\r\n")); err != nil { conn.Close() return nil, err } return conn, nil }Tests that need successful auth can use:
conn, err := suite.authenticateTelnet("root", "test") suite.Require().NoError(err) defer conn.Close() response, err := readTelnetUntil(conn, "root@test-server:~$") // ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…nd handling
All Submissions:
New Feature Submissions:
Changes to Core Features:
Summary by CodeRabbit
New Features
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.