interactsh v1.3.0

[dogancanbakir]

do not merge pls

Summary by CodeRabbit

• New Features

  • Multi-IP support (IPv4/IPv6) via CLI, eviction-strategy flag (sliding/fixed), and configurable default HTTP response/index/banner.

• Improvements

  • Enforced minimums for correlation ID/nonce lengths, prioritized default HTTP responses, safer resource close/shutdown handling, consistent address/log formatting, refined DNS/HTTP behavior for multi-IP setups, and updated output/help text.

• Tests

  • Added DNS tests for IPv4/IPv6 scenarios and tests for sliding/fixed eviction strategies.

• Chores

  • Toolchain and dependency upgrades; README/examples updated for new flags and usage.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds multi‑IP (IPv4/IPv6) support and CLI parsing, introduces eviction strategy option (sliding/fixed), enforces correlation‑ID minima, switches storage to AES‑CTR with 32‑byte random keys, adds default HTTP response handling, tightens error handling, expands DNS tests, and upgrades Go toolchain/dependencies.

Changes

Cohort / File(s)	Summary
Docs & Manifest README.md, go.mod	README updated for IPv4/IPv6 and new flags (-ip, -es, -dhr); examples/formats adjusted. go.mod bumped to Go 1.24 and numerous dependency upgrades.
Server CLI & Entrypoints cmd/interactsh-server/main.go, cmd/interactsh-client/main.go, cmd/benchmark-server/duration-testing/bench.go	CLI accepts multiple -ip values and -es eviction flag; improved flag help text; safer resource close logging and standardized stdout formatting.
Options & Server API pkg/options/server_options.go, pkg/server/server.go, pkg/options/utils.go	IPAddress → IPAddresses (CLI slice → []net.IP), added EvictionStrategy and DefaultHTTPResponseFile, IP parsing/deduping, and version bumped to 1.3.0.
DNS server & tests pkg/server/dns_server.go, pkg/server/dns_server_test.go, pkg/server/util.go	DNS now answers A/AAAA and glue for all configured IPs; new helpers formatAddress, parseIPList, uniqueIPs; extensive DNS tests added.
HTTP server pkg/server/http_server.go	Adds defaultResponse (from DefaultHTTPResponseFile) prioritized in default handler; uses formatAddress for listen targets; normalizes response writes.
Storage: eviction & crypto pkg/storage/option.go, pkg/storage/storagedb.go, pkg/storage/storagedb_test.go, pkg/storage/util.go	New EvictionStrategy enum and option; storagedb uses crypto/rand 32‑byte AES keys, AES‑CTR mode, conditional AES encryption for disk, and sliding vs fixed eviction behavior; tests added.
Client, errors & settings pkg/client/client.go, pkg/storage/error.go, pkg/settings/default.go	Switched to errkit for error wrapping/creation, added correlation‑ID minimum constants, and CLI validation for correlation ID lengths.
Resource closes & server cleanup examples/client.go, internal/runner/healthcheck.go, pkg/filewatcher/filewatcher.go, pkg/server/responder_server.go, pkg/server/smb_server.go	Many Close()/RemoveAll() calls now ignore or log close errors (_ = ...), example defers surfaced errors, and unused per‑server ipAddress fields removed.
Address formatting utility pkg/server/smtp_server.go, pkg/server/ldap_server.go, pkg/server/util.go	Replaced manual "%s:%d" listener formatting with formatAddress(host, port) using net.JoinHostPort.
ACME & cert handling pkg/server/acme/acme_certbot.go, pkg/server/acme/records_store.go	Certmagic DNS01Solver configured via DNSManager; record comparisons now use RR-derived fields (Type, Name, Data, TTL).

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant DNS_Server
  participant Config
  Client->>DNS_Server: Query A/AAAA for name
  DNS_Server->>Config: Read configured IPAddresses (IPv4/IPv6 list)
  DNS_Server->>DNS_Server: parseIPList() / uniqueIPs()
  DNS_Server->>DNS_Server: build answer records (A/AAAA) and glue
  DNS_Server-->>Client: Return answers (multiple A/AAAA) and glue records

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐇 I hopped through v4 and v6 tonight,

Sliding or fixed, the caches hum so bright,
Thirty‑two bytes guard the moonlit store,
Servers answer on every open door,
I twitched my nose — the tests all passed in sight.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 35.71% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'interactsh v1.3.0' is a version bump label that partially relates to the changeset. While the PR updates the version constant from 1.2.4 to 1.3.0, the title does not convey the substantial feature additions (IPv4/IPv6 support, eviction strategies, default HTTP responses, error handling improvements) that comprise the actual changes.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI Agents

In @pkg/filewatcher/filewatcher.go:
- Line 52: The call that currently swallows the Close error ("_ = r.Close()")
should capture and log any returned error for observability; replace it with
error handling like "if err := r.Close(); err != nil { /* log the error with
context */ }" and use the module's existing logger (e.g., filewatcher logger
instance such as fw.logger or a package logger) or log.Printf if none exists,
including contextual info (file/path or watcher id) so repeated close failures
are visible.

🧹 Nitpick comments (7)

internal/runner/healthcheck.go (1)

46-46: Optionally log close errors for consistency.

The close errors are being ignored on health check connections. While this is acceptable for short-lived test connections, logging these errors could provide additional diagnostic information without affecting the health check logic.

Also applies to: 55-55, 64-64

pkg/storage/util.go (1)

39-39: Error message capitalization changed.

The error message was changed from "Key type is not RSA" to "key type is not RSA". While this is a minor stylistic change, consider maintaining consistency with other error messages in the codebase.

pkg/server/acme/acme_certbot.go (1)

27-29: Consider logging cleanup errors instead of silently discarding them.

The error from certmagic.CleanStorage is now explicitly ignored. While cleanup errors may not be critical to operation, they could indicate storage issues (e.g., permission problems, disk full) that would be valuable to log for debugging and monitoring purposes.

🔎 Proposed enhancement to log errors

 func CleanupStorage() {
 	cleanupOptions := certmagic.CleanStorageOptions{OCSPStaples: true}
-	_ = certmagic.CleanStorage(context.Background(), certmagic.Default.Storage, cleanupOptions)
+	if err := certmagic.CleanStorage(context.Background(), certmagic.Default.Storage, cleanupOptions); err != nil {
+		gologger.Warning().Msgf("Certificate storage cleanup failed: %s", err)
+	}
 }

pkg/server/http_server.go (1)

282-282: Explicitly ignoring write errors is acceptable for HTTP responses.

The code now explicitly ignores return values from fmt.Fprint/fmt.Fprintf using _, _. In HTTP handlers, write failures (e.g., client disconnects) are typically non-fatal and handled by the HTTP stack, so this is acceptable practice.

However, consider logging write errors for debugging purposes if observability is a concern.

Also applies to: 284-284, 287-287, 289-289, 292-292, 299-299

pkg/server/dns_server_test.go (1)

188-202: Test helper silently ignores invalid IPs.

The newTestOptions helper silently skips invalid IP strings without logging or failing. While acceptable for tests where inputs are controlled, consider adding a t.Helper() call and optionally a t.Fatalf for unexpected parse failures to catch test setup issues earlier.

🔎 Suggested improvement

-func newTestOptions(ips []string, listenIP string) *Options {
+func newTestOptions(t *testing.T, ips []string, listenIP string) *Options {
+	t.Helper()
 	parsed := make([]net.IP, 0, len(ips))
 	for _, ip := range ips {
 		parsedIP := net.ParseIP(ip)
 		if parsedIP != nil {
 			parsed = append(parsed, parsedIP)
+		} else if ip != "" {
+			t.Fatalf("invalid test IP: %s", ip)
 		}
 	}

pkg/server/dns_server.go (1)

424-449: Duplicate uniqueIPs function.

This is the second implementation of uniqueIPs (also in pkg/options/server_options.go). This implementation is slightly better (uses struct{} instead of bool, includes nil check), but duplication should be addressed.

Consider extracting to a shared utility package to eliminate duplication.

pkg/options/server_options.go (1)

118-132: Duplicate uniqueIPs implementation.

This function duplicates logic in pkg/server/dns_server.go. Consider extracting to a shared utility (e.g., pkg/util/ip.go) to maintain consistency and reduce duplication.

#!/bin/bash
# Find all uniqueIPs implementations
rg -n "func uniqueIPs" --type go

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0eb8a06 and 590d76b.

⛔ Files ignored due to path filters (2)

• .github/workflows/stale.yml is excluded by !**/*.yml
• go.sum is excluded by !**/*.sum

📒 Files selected for processing (27)

• README.md
• cmd/benchmark-server/duration-testing/bench.go
• cmd/interactsh-client/main.go
• cmd/interactsh-server/main.go
• examples/client.go
• go.mod
• internal/runner/healthcheck.go
• pkg/client/client.go
• pkg/filewatcher/filewatcher.go
• pkg/options/server_options.go
• pkg/server/acme/acme_certbot.go
• pkg/server/acme/records_store.go
• pkg/server/dns_server.go
• pkg/server/dns_server_test.go
• pkg/server/http_server.go
• pkg/server/ldap_server.go
• pkg/server/responder_server.go
• pkg/server/server.go
• pkg/server/smb_server.go
• pkg/server/smtp_server.go
• pkg/server/util.go
• pkg/settings/default.go
• pkg/storage/error.go
• pkg/storage/option.go
• pkg/storage/storagedb.go
• pkg/storage/storagedb_test.go
• pkg/storage/util.go

🧰 Additional context used

🧬 Code graph analysis (5)

pkg/storage/storagedb_test.go (2)

pkg/storage/storagedb.go (1)

• New (34-74)

pkg/storage/option.go (4)

• Options (12-17)
• EvictionStrategy (5-5)
• EvictionStrategySliding (8-8)
• EvictionStrategyFixed (9-9)

pkg/server/dns_server_test.go (3)

pkg/server/dns_server.go (1)

• NewDNSServer (36-65)

pkg/server/acme/records_store.go (1)

• CertificateAuthority (98-98)

pkg/server/server.go (1)

• Options (38-118)

pkg/client/client.go (3)

pkg/storage/storagedb.go (1)

• New (34-74)

pkg/settings/default.go (2)

• CorrelationIdLengthMinimum (6-6)
• CorrelationIdNonceLengthMinimum (7-7)

pkg/storage/error.go (1)

• ErrCorrelationIdNotFound (5-5)

cmd/interactsh-client/main.go (1)

pkg/settings/default.go (4)

• CorrelationIdLengthDefault (4-4)
• CorrelationIdLengthMinimum (6-6)
• CorrelationIdNonceLengthDefault (5-5)
• CorrelationIdNonceLengthMinimum (7-7)

pkg/storage/storagedb.go (3)

pkg/storage/option.go (3)

• EvictionStrategy (5-5)
• EvictionStrategyFixed (9-9)
• EvictionStrategySliding (8-8)

pkg/storage/types.go (1)

• CorrelationData (20-30)

pkg/storage/util.go (1)

• AESEncrypt (43-59)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: release-test
• GitHub Check: Test Builds (ubuntu-latest)
• GitHub Check: Test Builds (windows-latest)
• GitHub Check: Test Builds (macOS-latest)

🔇 Additional comments (52)

cmd/benchmark-server/duration-testing/bench.go (1)

112-114: Excellent improvement to error handling!

Adding error checking and logging for resp.Body.Close() improves observability and follows best practices for HTTP response handling.

examples/client.go (1)

17-21: Excellent demonstration of proper error handling!

The additions of error handling wrappers for Close(), StartPolling(), StopPolling(), and resp.Body.Close() transform this into a better example that demonstrates proper resource management patterns. Using panic in example code is appropriate for clarity.

Also applies to: 23-27, 28-32, 40-42

pkg/storage/util.go (1)

54-54: Critical: Verify backward compatibility for encryption mode change.

The encryption mode has been changed from CFB (cipher.NewCFBEncrypter) to CTR (cipher.NewCTR). These cipher modes are incompatible - data encrypted with CFB cannot be decrypted with CTR. This is a breaking change that will make any existing encrypted data unreadable unless there's a migration strategy in place.

Please verify:

1. Has the corresponding decryption function been updated to use CTR mode?
2. Is there a migration strategy for existing encrypted data?
3. Are there tests that validate decryption of data encrypted with the new mode?
4. Will this break any existing stored encrypted data in production?

pkg/storage/error.go (1)

3-5: LGTM! Error handling standardization.

The migration from the standard errors package to errkit aligns with the project-wide error handling improvements seen throughout this PR.

go.mod (2)

10-36: Verify compatibility of updated dependencies.

Multiple direct dependencies have been updated, including:

• certmagic: v0.25.0 (significant upgrade)
• retryablehttp-go: v1.3.2
• utils: v0.8.0
• Various other packages

Ensure that:

1. Breaking changes in updated dependencies have been addressed
2. The certmagic v0.25.0 API changes (seen in acme_certbot.go) are correctly implemented
3. Integration tests cover the updated dependency behavior

---

3-5: Verify Go 1.24.0 compatibility and test coverage.

The Go version has been upgraded from 1.21 to 1.24.0 with toolchain go1.24.2. While Go 1.24 honors the Go 1 compatibility promise and most programs should continue to work unchanged, it does introduce changes worth checking:

• Generic type aliases are now fully supported (can be disabled via GOEXPERIMENT=noaliastypeparams if needed)
• Stricter rules for methods on cgo-generated types
• New Swiss-table map and mutex implementations (changes in low-level behavior and performance)
• New tool directives in go.mod

Ensure that:

1. All tests pass with Go 1.24.0
2. CI/CD pipelines are updated to use the new Go version
3. Code using cgo receivers or relying on previous map/mutex behavior is reviewed
4. Dependencies are compatible with Go 1.24.0

pkg/server/ldap_server.go (1)

62-62: LGTM! Address formatting standardization supports IPv6.

The use of formatAddress helper (which wraps net.JoinHostPort) provides consistent address formatting across all servers and properly handles IPv6 addresses with bracket notation. This aligns with the IPv6 support additions in this PR.

pkg/server/acme/records_store.go (1)

31-34: Verify that RR-based comparison maintains correct equality semantics.

The record comparison logic has been refactored to use RR() method results instead of direct field comparison. While this may provide better normalization, ensure that:

1. The RR() method consistently represents equivalent records
2. Edge cases (empty fields, case sensitivity, trailing dots in names) are handled correctly
3. Existing ACME DNS challenge flows continue to work as expected

pkg/server/acme/acme_certbot.go (1)

46-56: Configuration is correct for certmagic v0.25.0.

The DNS01Solver configuration properly uses the DNSManager struct to wrap DNSProvider and Resolvers, which is the required pattern in certmagic v0.25.0. No changes needed.

pkg/server/server.go (2)

5-5: LGTM!

The net import addition is necessary for the net.IP type used in the IPAddresses field.

---

41-42: Verify all references to the old IPAddress field have been updated to IPAddresses with the new type.

This field was changed from string to []net.IP and renamed to reflect the slice semantics. As a breaking API change to a public field, all code accessing Options.IPAddress must be updated to use Options.IPAddresses with the appropriate slice type handling.

pkg/server/util.go (2)

4-5: LGTM!

The added imports are necessary for the formatAddress function and are from the standard library.

---

24-26: LGTM!

The formatAddress helper centralizes address construction and correctly uses net.JoinHostPort, which properly handles IPv6 addresses with brackets.

pkg/settings/default.go (1)

6-7: LGTM!

The new minimum-length constants provide sensible lower bounds for correlation ID validation and follow the existing naming convention.

pkg/server/http_server.go (1)

80-81: LGTM!

The use of formatAddress for server address construction is consistent with the new helper and properly handles both IPv4 and IPv6 addresses.

pkg/server/smtp_server.go (1)

36-36: LGTM!

The use of formatAddress for SMTP server address construction is consistent with the refactoring across other server components and properly handles IPv6 addresses.

Also applies to: 44-44, 60-60

pkg/server/responder_server.go (2)

32-34: LGTM!

The constructor simplification aligns well with the broader refactor to centralize IP address handling via options rather than per-server fields.

---

109-114: LGTM!

Ignoring cleanup errors in the Close() method is a reasonable approach, preventing secondary failures during shutdown.

pkg/storage/storagedb_test.go (2)

163-181: LGTM!

The fixed eviction strategy test correctly verifies that access does not extend expiration time, and the item expires after the original TTL.

---

113-113: [rewritten review comment]
[classification tag]

pkg/storage/option.go (1)

5-26: LGTM!

The eviction strategy enum and its integration into the Options struct is well-designed. The inline comments clearly distinguish between sliding (expire-after-access) and fixed (expire-after-write) strategies, and the default of sliding eviction is a reasonable choice.

pkg/server/smb_server.go (2)

32-35: LGTM!

The constructor simplification is consistent with the broader refactor to centralize IP address handling, matching the pattern in responder_server.go.

---

121-126: LGTM!

Ignoring cleanup errors in the Close() method is appropriate, consistent with the same change in responder_server.go.

README.md (5)

28-43: LGTM!

The features list accurately reflects the new IPv4 and IPv6 support added in this PR.

---

349-349: LGTM!

The eviction-strategy flag documentation clearly describes the options (sliding, fixed) and default value, aligning with the implementation in pkg/storage/option.go.

---

345-345: LGTM!

The IP flag documentation correctly reflects the change to support multiple IP addresses (both IPv4 and IPv6) with comma-separated values.

---

500-527: LGTM!

The new IPv6 section provides clear documentation with appropriate examples using reserved documentation IP addresses (192.0.2.1 for IPv4 and 2001:db8::1 for IPv6).

---

69-70: Verify minimum length constraints match the implementation.

The documentation now specifies a minimum length of 3 for both correlation-id-length and correlation-id-nonce-length flags. Ensure these constraints are enforced in the code.

pkg/server/dns_server_test.go (3)

12-22: Comprehensive IPv4/IPv6 DNS test coverage.

Good test structure verifying both Answer and Extra (glue) sections for mixed IPv4/IPv6 configurations. The use of documentation IP ranges (192.0.2.x for IPv4, 2001:db8::/32 for IPv6) follows RFC 5737 and RFC 3849 best practices.

---

46-55: Good edge case coverage for empty IP configuration.

This test ensures graceful handling when no IPs are configured, preventing potential nil pointer issues in production.

---

172-186: Helper function correctly handles A and AAAA record type assertions.

The hasRecord helper properly uses type switches and allows both existence checks (empty expectedValue) and value matching. This is clean and reusable across tests.

pkg/options/server_options.go (1)

64-77: IP parsing and deduplication logic is correct.

The parsing handles invalid IPs gracefully with debug-level warnings, avoiding crashes for user input errors. The deduplication step ensures consistent behavior.

pkg/client/client.go (4)

110-116: Good input validation for correlation ID lengths.

Adding minimum length validation prevents misconfiguration that could break correlation ID functionality. Error messages clearly indicate the required minimums.

---

391-395: Improved error handling with errkit.Is for sentinel errors.

Using errkit.Is for comparing sentinel errors (errAuth, storage.ErrCorrelationIdNotFound) is the correct approach for wrapped errors.

---

481-483: Good defensive check for empty TLD data.

Skipping empty data entries prevents unnecessary unmarshaling attempts and potential errors.

---

693-700: Breaking change: CFB to CTR cipher mode.

The switch from CFB to CTR mode is a breaking change. Existing clients cannot decrypt data encrypted with the old CFB mode, and old servers cannot decrypt CTR-encrypted data from new clients. Ensure:

1. This is intentional and documented
2. Migration path exists for existing deployments
3. Version compatibility is handled

pkg/storage/storagedb.go (4)

39-47: Clean eviction strategy implementation.

The switch statement with fallthrough for default to sliding strategy is a clear pattern. Using ExpireAfterWrite for fixed and ExpireAfterAccess for sliding correctly implements the expected cache behaviors.

---

109-122: Improved AES key generation using crypto/rand.

Using crypto/rand for 32-byte AES key generation is cryptographically superior to the previous UUID-based approach. The error handling for random generation is properly implemented.

---

138-140: Good guard clause for empty data.

Returning early for empty data prevents unnecessary cache lookups and potential nil-related issues.

---

151-159: Verify the intentionality of plaintext storage in the conditional encryption path.

When value.AESKey is empty/nil, data is stored as plaintext. Confirm whether this is expected behavior (e.g., only for SetID non-authenticated sessions) or a potential security issue. If encryption should always be enforced, the missing AES key case should either raise an error or use a default encryption key.

cmd/interactsh-client/main.go (3)

55-56: Improved CLI flag documentation.

Including minimum and default values in the flag descriptions helps users understand constraints without needing to consult external documentation.

---

148-152: Proper deferred file close with error handling.

Logging errors on file close is better than silently ignoring them, as write buffers may fail to flush.

---

272-277: Explicit error ignoring for stdout writes.

Using _, _ = to explicitly ignore errors from stdout writes is acceptable for CLI tools where stdout failures are typically unrecoverable. The JSON output path correctly writes to both stdout and the output file when specified.

pkg/server/dns_server.go (4)

363-381: IPv4/IPv6 detection logic in appendAnswerRecord.

The logic uses ip.To4() to detect IPv4, which is correct. However, for IPv6, ip.To16() will return non-nil for both IPv4 and IPv6 addresses (IPv4 addresses are represented as IPv4-mapped IPv6). Since this is in an else if after the IPv4 check, it works correctly, but the comment could clarify this.

---

383-395: Glue records correctly include both A and AAAA records.

The implementation properly iterates through all configured IPs and appends appropriate glue records based on IP family. Using continue after IPv4 prevents falling through to IPv6 check.

---

397-422: Flexible IP list parsing.

The parseIPList function handles comma, semicolon, and whitespace separators, making it robust for various input formats. Invalid IPs are silently skipped, which is appropriate for DNS record lookups.

---

482-486: Improved file close handling with error logging.

Wrapping the file close in a deferred function with error logging prevents silent failures during cleanup.

cmd/interactsh-server/main.go (5)

147-152: Consistent validation for correlation ID lengths.

Server-side validation mirrors the client-side validation, ensuring configuration errors are caught early with clear error messages.

---

234-244: Robust eviction strategy parsing.

The switch statement with strings.ToLower handles case-insensitive input, and the default case provides a clear error message listing valid options. This is user-friendly and prevents silent misconfiguration.

---

354-358: Improved LDAP server cleanup.

The deferred close with error logging ensures cleanup issues are visible. This pattern is consistently applied throughout the PR.

---

184-188: Multi-IP configuration logging.

Logging the configured IP addresses helps with debugging and operational visibility. Using strings.Join provides a clean comma-separated output.

---

446-448: Proper address formatting with net.JoinHostPort.

Using net.JoinHostPort correctly handles IPv6 addresses (adding brackets when needed), which is essential for the new IPv6 support.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Consider logging the close error for debugging purposes.

While ignoring the close error may be acceptable in this monitoring loop, repeated close failures could lead to file descriptor leaks over time. Consider at least logging the error for observability.

🔎 Suggested improvement

-			_ = r.Close()
+			if err := r.Close(); err != nil {
+				gologger.Warning().Msgf("Failed to close file: %s", err)
+			}

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	_ = r.Close()
	if err := r.Close(); err != nil {
	gologger.Warning().Msgf("Failed to close file: %s", err)
	}

🤖 Prompt for AI Agents

In @pkg/filewatcher/filewatcher.go at line 52, The call that currently swallows
the Close error ("_ = r.Close()") should capture and log any returned error for
observability; replace it with error handling like "if err := r.Close(); err !=
nil { /* log the error with context */ }" and use the module's existing logger
(e.g., filewatcher logger instance such as fw.logger or a package logger) or
log.Printf if none exists, including contextual info (file/path or watcher id)
so repeated close failures are visible.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Clarify the sliding eviction test logic.

The test appears to have contradictory expectations. In sliding eviction (expire-after-access), each access should extend the expiration time. However:

• Line 149: First access at t≈50ms should extend expiration to t≈150ms
• Line 154: Second access at t≈110ms should extend expiration to t≈210ms (under true sliding semantics)
• Line 159: The test expects expiration at t≈220ms, which would only make sense if the second access didn't extend the TTL

The comment "Should be expired after full TTL despite access" (line 157) contradicts sliding window behavior. Either the test logic needs correction or the eviction strategy implementation differs from typical sliding window semantics.

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In @go.mod:
- Around line 3-5: Add an explicit PR/commit note documenting the Go toolchain
bump from 1.21 → 1.24.0 (toolchain go1.24.2) and list the concurrently upgraded
dependencies (certmagic, uuid, libdns, retryablehttp-go, utils, xid) plus the
rationale for the upgrade; then run and verify the full CI matrix and local
vet/compile checks (run go vet / go test / go build under Go 1.24.2) to surface
any new vet diagnostics or compiler errors and address any failures before
merging so the pipeline passes with the new toolchain.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7fc788c and ee56860.

⛔ Files ignored due to path filters (1)

• go.sum is excluded by !**/*.sum

📒 Files selected for processing (1)

• go.mod

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Test Builds (macOS-latest)
• GitHub Check: Test Builds (windows-latest)
• GitHub Check: Test Builds (ubuntu-latest)
• GitHub Check: release-test

🔇 Additional comments (2)

go.mod (2)

24-24: No action needed. The version change is correct: retryablehttp-go was bumped from 1.3.2 to 1.3.3, which is a standard patch update as indicated in the PR objectives and confirmed by dependabot metadata.

Likely an incorrect or invalid review comment.

---

39-150: Use govulncheck instead of GitHub API to detect Go vulnerabilities.

The suggested GitHub GraphQL queries are not the recommended approach for checking Go package vulnerabilities. Go provides the official govulncheck tool (golang.org/x/vuln), which integrates with the authoritative Go vulnerability database (vuln.go.dev) and reports only vulnerabilities that can affect your code.

Instead, run:

go install golang.org/x/vuln/cmd/govulncheck@latest
govulncheck ./...

The go mod verify and go mod tidy suggestions are fine for ensuring module integrity, but they don't detect security vulnerabilities.

Likely an incorrect or invalid review comment.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for Go version requirements in the codebase
echo "=== Checking for go.mod and go.sum ==="
ls -la go.* 2>/dev/null || echo "No go.* files found"

echo -e "\n=== Checking for Go version build constraints ==="
rg -n '//go:build|//\+build' --type go -c | head -20

echo -e "\n=== Checking for Go version mentions in documentation ==="
fd -e md -e txt -e rst -e asciidoc | xargs grep -l "go\|Go\|golang" 2>/dev/null | head -10

echo -e "\n=== Checking go.mod first 40 lines ==="
head -40 go.mod

Repository: projectdiscovery/interactsh

Length of output: 1754

---

🌐 Web query:

Go 1.24.0 release notes breaking changes

💡 Result:

Here are the notable changes in Go 1.24 that can affect compatibility or break existing code (from the official release notes). [1][2]

• Generic type aliases enabled: type aliases may now have type parameters (aliastypeparams). This can change type resolution/overloads in some code; the feature can be disabled with GOEXPERIMENT=noaliastypeparams (but that flag is scheduled for removal in Go 1.25). [1][2]
• New vet diagnostics (applied when the module/go version is >= 1.24): e.g., the printf check for fmt.Printf(s) and the tests analyzer — these can surface errors/warnings and cause CI/test failures when upgrading the toolchain. [1]
• Compiler stricter checks for cgo-generated receiver types: the compiler now reports an error if a method receiver denotes a cgo-generated type, even if referenced indirectly through an alias (previously some aliases could circumvent this). This can turn previously compiling code into an error. [1]
• Runtime/map & mutex implementation changes (Swiss Tables map; new runtime-internal mutex): these improve performance but change internal behavior; they can be disabled at build time with GOEXPERIMENT=noswissmap and GOEXPERIMENT=nospinbitmutex if needed. Some programs that relied on subtle map/mutex implementation details might observe differences. [1]
• Bootstrap requirement: building the toolchain now requires Go 1.22.6 or later (affects people bootstrapping from older Go versions). [1]

References:
[1] Go 1.24 Release Notes (official).
[2] Go Blog: "Go 1.24 is released!"

---

🌐 Web query:

Go 1.24 vs Go 1.21 compatibility changes

💡 Result:

Summary of compatibility-relevant changes from Go 1.21 → Go 1.24

• Language

  • Go 1.24: full support for parameterized (generic) type aliases (alias declarations may have type parameters). This is a new language feature (can be disabled with GOEXPERIMENT=noaliastypeparams until removed in 1.25). [2][3]

• Tooling / modules / toolchains

  • Go 1.21: introduced stricter interpretation of the go line (go.mod / go.work) and GODEBUG-based compatibility behavior so toolchains can preserve older behavior when needed. [1]
  • Go 1.24: adds explicit tool dependency support (tool directive in go.mod), a -tool flag for go get, a tool meta-pattern, and caching/running of tool binaries via the go tool / build cache. These changes change how projects track and run build/test tooling (replaces the tools.go blank-import workaround). [2][3]

• Migration / compatibility notes

  • Fixes/behavior changes that are non-breaking under the Go compatibility policy may still change program behaviour; Go 1.21’s GODEBUG/toolchain handling helps preserve older behavior when the module/workspace declares an older go version. Upgrading to 1.24 may expose bug fixes or new behaviour unless your module/workspace go line pins the older version or you use the compatibility controls. [1][2]

• Other notable platform/security/runtime updates (may affect builds)

  • 1.24 includes various runtime/toolchain updates (including FIPS-related and platform build adjustments in some vendor builds) — these can affect builds on certain distributions or when relying on system crypto providers. Check vendor-specific notes if you use a downstream build (e.g., Microsoft build notes). [4]

If you want, I can:

• list the exact release-note links and the specific Go issue/CL numbers for each change, or
• scan your repo for things likely to break when moving from go 1.21 to 1.24.

Sources: Go 1.21 release notes; Go 1.24 changelog summaries and vendor notes. [1][2][3][4]

References
[1] Go 1.21 release notes (go.dev).
[2] Go 1.24 new-features summary.
[3] Go 1.24 highlights / SUSE advisory.
[4] Microsoft blog post on Go 1.24 (Microsoft build / FIPS notes).

---

Go toolchain upgrade (1.21 → 1.24.0) requires explicit PR documentation and CI verification.

The Go version has been upgraded from 1.21 to 1.24.0 (with toolchain go1.24.2) but this significant change is not mentioned in the PR objectives or commit message. Go 1.24 introduces new vet diagnostics and stricter compiler checks that may surface warnings or failures in CI pipelines. The dependencies have also been updated concurrently (certmagic, uuid, libdns, retryablehttp-go, utils, xid to major versions).

Before merging, add an explicit note to the PR description explaining the Go toolchain upgrade rationale, and verify that the new vet rules do not introduce CI build failures.

🤖 Prompt for AI Agents

In @go.mod around lines 3 - 5, Add an explicit PR/commit note documenting the Go
toolchain bump from 1.21 → 1.24.0 (toolchain go1.24.2) and list the concurrently
upgraded dependencies (certmagic, uuid, libdns, retryablehttp-go, utils, xid)
plus the rationale for the upgrade; then run and verify the full CI matrix and
local vet/compile checks (run go vet / go test / go build under Go 1.24.2) to
surface any new vet diagnostics or compiler errors and address any failures
before merging so the pipeline passes with the new toolchain.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's examine the go.mod file to confirm the version changes
cat go.mod

Repository: projectdiscovery/interactsh

Length of output: 7303

---

🏁 Script executed:

# Search for imports of the mentioned packages
rg --type go -i "github.com/caddyserver/certmagic|github.com/google/uuid|github.com/projectdiscovery/retryablehttp-go|github.com/projectdiscovery/utils|github.com/rs/xid|github.com/projectdiscovery/libdns" | head -30

Repository: projectdiscovery/interactsh

Length of output: 2217

---

🏁 Script executed:

# Check for specific usage patterns that might indicate breaking changes
echo "=== certmagic usage ==="
rg -n --type go 'certmagic\.' | head -20

echo -e "\n=== uuid usage ==="
rg -n --type go 'uuid\.' | head -20

echo -e "\n=== xid usage ==="
rg -n --type go 'xid\.' | head -20

Repository: projectdiscovery/interactsh

Length of output: 2703

---

Verify compatibility with version updates, particularly libdns v0→v1 transition.

The go.mod contains several dependency updates. While most are minor version bumps within the same major version (uuid v1→v1, xid v1→v1, retryablehttp-go v1→v1), two merit specific attention:

• libdns: v0.2.1 → v1.1.1 (major version transition from v0 to v1)
• certmagic and utils: v0.x updates which may include breaking changes per semver

Confirm that any breaking API changes have been addressed in the codebase, particularly for the libdns v1 migration, and that these updates do not introduce regressions.

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cmd/interactsh-server/main.go (1)

155-190: Fix outbound IP string handling (currently %s on net.IP can print raw bytes)
Compute outboundIPStr := outboundIP.String() (only if non-nil) and use it consistently for logging and bindable-address selection.

Proposed diff

 	if len(cliOptions.IPAddresses) == 0 && cliOptions.ListenIP == "0.0.0.0" {
 		publicIP, _ := getPublicIP()
 		outboundIP, _ := iputil.GetSourceIP("scanme.sh")
+		outboundIPStr := ""
+		if outboundIP != nil {
+			outboundIPStr = outboundIP.String()
+		}

 		if publicIP == "" && outboundIP == nil {
 			gologger.Fatal().Msgf("Could not determine public IP address\n")
 		}
@@
 		gologger.Info().Msgf("Public IP: %s\n", publicIP)
-		gologger.Info().Msgf("Outbound IP: %s\n", outboundIP)
+		gologger.Info().Msgf("Outbound IP: %s\n", outboundIPStr)
 		// it's essential to be able to bind to cliOptions.DnsPort on any of the two ips
-		bindableIP, err := iputil.GetBindableAddress(cliOptions.DnsPort, publicIP, outboundIP.String())
+		bindableIP, err := iputil.GetBindableAddress(cliOptions.DnsPort, publicIP, outboundIPStr)
@@
 		cliOptions.ListenIP = bindableIP
 		cliOptions.IPAddresses = append(cliOptions.IPAddresses, publicIP)
 	}

🧹 Nitpick comments (2)

pkg/options/server_options.go (2)

11-62: Trim/validate IP strings before net.ParseIP to avoid accidental drops
net.ParseIP is strict; if --ip / config values include whitespace, they’ll be treated as invalid and silently ignored unless Debug is on. Consider trimming and skipping empty entries defensively.

Proposed diff

 package options

 import (
 	"net"
+	"strings"

 	"github.com/projectdiscovery/goflags"
 	"github.com/projectdiscovery/gologger"
 	"github.com/projectdiscovery/interactsh/pkg/server"
 )
@@
 func (cliServerOptions *CLIServerOptions) AsServerOptions() *server.Options {
 	var ipAddresses []net.IP

 	for _, ipAddress := range cliServerOptions.IPAddresses {
+		ipAddress = strings.TrimSpace(ipAddress)
+		if ipAddress == "" {
+			continue
+		}
 		parsedIP := net.ParseIP(ipAddress)
 		if parsedIP != nil {
 			ipAddresses = append(ipAddresses, parsedIP)
 		} else {
 			if cliServerOptions.Debug {
 				gologger.Warning().Msgf("Invalid IP address '%s' will be ignored\n", ipAddress)
 			}
 		}
 	}

Also applies to: 64-117

---

120-134: Use map[string]struct{} + prealloc in uniqueIPs
Small perf/clarity win; behavior stays the same.

Proposed diff

 func uniqueIPs(ips []net.IP) []net.IP {
-	seen := make(map[string]bool)
-	result := []net.IP{}
+	seen := make(map[string]struct{}, len(ips))
+	result := make([]net.IP, 0, len(ips))

 	for _, ip := range ips {
 		key := ip.String()
-		if !seen[key] {
-			seen[key] = true
-			result = append(result, ip)
-		}
+		if _, ok := seen[key]; ok {
+			continue
+		}
+		seen[key] = struct{}{}
+		result = append(result, ip)
 	}

 	return result
 }

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ee56860 and 60285b0.

📒 Files selected for processing (5)

• README.md
• cmd/interactsh-server/main.go
• pkg/options/server_options.go
• pkg/server/http_server.go
• pkg/server/server.go

🚧 Files skipped from review as they are similar to previous changes (3)

• pkg/server/server.go
• pkg/server/http_server.go
• README.md

🧰 Additional context used

🧬 Code graph analysis (1)

pkg/options/server_options.go (2)

pkg/storage/option.go (2)

• EvictionStrategy (5-5)
• Options (12-17)

pkg/server/server.go (1)

• Options (38-120)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)

• GitHub Check: Test Builds (macOS-latest)
• GitHub Check: Test Builds (windows-latest)
• GitHub Check: Test Builds (ubuntu-latest)
• GitHub Check: release-test

🔇 Additional comments (6)

cmd/interactsh-server/main.go (6)

46-63: CLI additions look consistent (multi-IP, eviction strategy, default response, min-length help text)
Flags/wiring align with the new CLIServerOptions fields and settings constants.

Also applies to: 65-77

---

148-153: Good guardrails: enforce minimum correlation-id lengths
Prevents weak/invalid correlation IDs early with clear errors.

---

236-251: Eviction strategy parsing + propagation looks solid
Clear mapping + fail-fast on invalid values.

---

355-359: Nice: log LDAP shutdown failures instead of ignoring them

---

447-454: Good: net.JoinHostPort for correct IPv6 listen-address logging

---

477-479: Good: handle pprofServer.Close() errors

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@pkg/storage/storagedb.go`:
- Around line 151-159: The current disk-write path silently stores plaintext
when value.AESKey is empty (inside the s.Options.UseDisk() branch where ct :=
string(data) and AESEncrypt is called), which can cause silent security
downgrade; update the code to fail-fast or require an explicit plaintext opt-in:
when s.Options.UseDisk() is true and len(value.AESKey) == 0, return an error (or
check an explicit flag like value.AllowPlaintext and require it) instead of
continuing to write unencrypted data, and apply the same guard where AESKey is
checked in the later block (the other occurrence you noted around lines 189-197)
to ensure consistent behavior.
- Around line 128-131: SetID currently creates CorrelationData with an empty
AESKey, which causes AESEncrypt to fail in the in-memory path when
getInteractions (used by GetInteractionsWithId) tries to encrypt cached entries;
modify getInteractions to check correlationData.AESKey and, if it's empty (len
== 0), return correlationData.Data unencrypted instead of calling AESEncrypt so
token/domain records created by SetID are returned correctly in-memory.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Find SetID usage and related retrieval paths.
rg -nP --type=go -C3 '\.SetID\s*\('
rg -nP --type=go -C3 'GetInteractionsWithId\s*\('
rg -nP --type=go -C3 'UseDisk\s*\('
rg -nP --type=go -C3 'DefaultOptions'

Repository: projectdiscovery/interactsh

Length of output: 9646

---

🏁 Script executed:

#!/bin/bash
# Get SetID implementation
sed -n '128,145p' pkg/storage/storagedb.go

# Get GetInteractionsWithId implementation
sed -n '231,245p' pkg/storage/storagedb.go

# Search for getInteractions (lowercase) which the comment mentions
rg -nP --type=go 'func \(.*\) getInteractions\s*\(' -A 20

Repository: projectdiscovery/interactsh

Length of output: 3276

---

🏁 Script executed:

#!/bin/bash
# Get the full getInteractions function
sed -n '280,330p' pkg/storage/storagedb.go

Repository: projectdiscovery/interactsh

Length of output: 1279

---

🏁 Script executed:

#!/bin/bash
# Find AESEncrypt implementation
rg -nP --type=go 'func AESEncrypt' -A 15

Repository: projectdiscovery/interactsh

Length of output: 1052

---

Fix in-memory encryption failure when SetID creates records with empty AESKey.

When UseDisk is false, GetInteractionsWithId calls getInteractions, which attempts to encrypt cached data by calling AESEncrypt with correlationData.AESKey. Since SetID creates records with an empty AESKey (see line 128-131), AESEncrypt will fail when trying to initialize the cipher with an empty key. The disk path avoids this by returning data without encryption, but the in-memory path will error.

SetID is used for token and domain records (cmd/interactsh-server/main.go lines 267, 275), which are later retrieved via GetInteractionsWithId (pkg/server/http_server.go lines 456, 463). These are explicitly intended to be unencrypted, so skip encryption when AESKey is empty:

Suggested fix

// in the in-memory case of getInteractions, before calling AESEncrypt:
if len(correlationData.AESKey) == 0 {
    return correlationData.Data, nil
}

🤖 Prompt for AI Agents

In `@pkg/storage/storagedb.go` around lines 128 - 131, SetID currently creates
CorrelationData with an empty AESKey, which causes AESEncrypt to fail in the
in-memory path when getInteractions (used by GetInteractionsWithId) tries to
encrypt cached entries; modify getInteractions to check correlationData.AESKey
and, if it's empty (len == 0), return correlationData.Data unencrypted instead
of calling AESEncrypt so token/domain records created by SetID are returned
correctly in-memory.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Avoid silent plaintext fallback when AESKey is missing.
If AESKey is unexpectedly empty (e.g., registration bug), data is stored unencrypted without any signal. Consider failing fast for correlation IDs or making plaintext mode explicit to avoid a silent downgrade.

🔒 Example guard (fail fast for encrypted flows)

-       ct := string(data)
-       if len(value.AESKey) > 0 {
+       ct := string(data)
+       if len(value.AESKey) == 0 {
+           return errors.New("missing AES key for encrypted interaction")
+       }
+       if len(value.AESKey) > 0 {
            var err error
            ct, err = AESEncrypt(value.AESKey, data)
            if err != nil {
                return errors.Wrap(err, "could not encrypt event data")
            }
        }

Also applies to: 189-197

🤖 Prompt for AI Agents

In `@pkg/storage/storagedb.go` around lines 151 - 159, The current disk-write path
silently stores plaintext when value.AESKey is empty (inside the
s.Options.UseDisk() branch where ct := string(data) and AESEncrypt is called),
which can cause silent security downgrade; update the code to fail-fast or
require an explicit plaintext opt-in: when s.Options.UseDisk() is true and
len(value.AESKey) == 0, return an error (or check an explicit flag like
value.AllowPlaintext and require it) instead of continuing to write unencrypted
data, and apply the same guard where AESKey is checked in the later block (the
other occurrence you noted around lines 189-197) to ensure consistent behavior.