Skip to content

Add STARTTLS support to testssl-inspector #197

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ahmedhxssam wants to merge 1 commit into
base: main
Choose a base branch
from
+72 −15
Open

Add STARTTLS support to testssl-inspector #197

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion plugins/connectors/testssl-inspector/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,4 +47,15 @@ See `commands/scan.md` for the full option set, and `skills/testssl-inspector-ex

## Scope

Only HTTPS endpoints in the current wrapper. `testssl.sh` itself supports `--starttls` for SMTP/IMAP/POP/FTP/etc.; if you need any of those, open an issue.
HTTPS endpoints by default. STARTTLS services are supported through the `--starttls=<proto>` flag for the following protocols with their default ports:

- `smtp` (25) — Simple Mail Transfer Protocol
- `imap` (143) — Internet Message Access Protocol
- `pop3` (110) — Post Office Protocol
- `ftp` (21) — File Transfer Protocol
- `ldap` (389) — Lightweight Directory Access Protocol
- `postgres` (5432) — PostgreSQL database
- `mysql` (3306) — MySQL database
- `smtps` (465) — SMTP Secure

When `--starttls=<proto>` is specified without an explicit port in the target, the target is automatically appended with the protocol's default port. If an explicit port is already present in the target, it is preserved. See `commands/scan.md` for examples.
18 changes: 14 additions & 4 deletions plugins/connectors/testssl-inspector/commands/scan.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,8 @@ node plugins/connectors/testssl-inspector/scripts/scan.js [options]

## Options

- `--target=host[:port]` — repeatable; or pass targets positionally. Port defaults to 443.
- `--target=host[:port]` — repeatable; or pass targets positionally. Port defaults to 443 for HTTPS, or the protocol's default port when `--starttls=<proto>` is used.
- `--starttls=<proto>` — enable STARTTLS for the specified protocol. Supported protocols: `smtp` (25), `imap` (143), `pop3` (110), `ftp` (21), `ldap` (389), `postgres` (5432), `mysql` (3306), `smtps` (465). When `--starttls=<proto>` is specified without an explicit port in the target, the target is automatically appended with the protocol's default port. For example, `--target=mail.example.com --starttls=smtp` becomes `mail.example.com:25`. If the target already includes an explicit port (e.g., `--target=mail.example.com:587 --starttls=smtp`), the explicit port is preserved.
- `--fast` — `testssl.sh --fast` (~3× faster, drops vulnerability checks).
- `--full` — full check set (default). Slower; includes CVE checks (Heartbleed, ROBOT, POODLE, BEAST, BREACH, SWEET32, FREAK, LOGJAM, DROWN, …).
- `--docker` / `--no-docker` — override the runner choice from the config.
Expand Down Expand Up @@ -69,18 +70,27 @@ Severity translation: testssl `FATAL`/`CRITICAL` → `critical`; `HIGH` → `hig
## Examples

```bash
# Fast scan of one endpoint
# Fast scan of one HTTPS endpoint
/testssl-inspector:scan --target=example.com --fast

# Full vulnerability scan against two endpoints, Docker runner
# Full vulnerability scan against two HTTPS endpoints, Docker runner
/testssl-inspector:scan --target=example.com --target=api.example.com:8443 --docker

# STARTTLS scan of SMTP service with default port
/testssl-inspector:scan --target=mail.example.com --starttls=smtp --output=json

# STARTTLS scan of SMTP service with explicit port
/testssl-inspector:scan --target=mail.example.com:587 --starttls=smtp --output=json

# STARTTLS scan of IMAP service
/testssl-inspector:scan --target=mail.example.com --starttls=imap --output=json

# Pipe summary into another step
/testssl-inspector:scan --target=example.com --output=json | jq '.counters'
```

## Targets and scope

- Only HTTPS endpoints. testssl supports `--starttls` for SMTP/IMAP/etc.; not currently wired into this wrapper — open an issue if you need it.
- HTTPS endpoints by default. STARTTLS services are supported through the `--starttls=<proto>` flag for protocols including SMTP, IMAP, POP3, FTP, LDAP, PostgreSQL, MySQL, and SMTPS.
- Scanning runs on the local machine and requires outbound network reach to the target. Not suitable for endpoints behind a private network unless the runner has access.
- Be courteous: testssl makes a couple hundred connections per target. Don't point it at infrastructure you don't own without authorization.
56 changes: 46 additions & 10 deletions plugins/connectors/testssl-inspector/scripts/scan.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,19 @@ const SCF_CACHE_TTL_MS = 7 * 24 * 60 * 60 * 1000;

const EXIT = { OK: 0, USAGE: 2, TOOL_UNAVAILABLE: 3, PARTIAL: 4, NOT_CONFIGURED: 5 };

const STARTTLS_PORTS = Object.freeze({
smtp: 25,
imap: 143,
pop3: 110,
ftp: 21,
ldap: 389,
postgres: 5432,
mysql: 3306,
smtps: 465,
});

const SUPPORTED_STARTTLS = Object.keys(STARTTLS_PORTS).join(', ');
Comment on lines +35 to +46

@qodo-code-review qodo-code-review Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. Node script in connector plugin 📘 Rule violation ⌂ Architecture

The modified plugins/connectors/testssl-inspector/scripts/scan.js is a Node.js entrypoint located
under a non-allowed (non-persona) plugin directory. This violates the requirement that Node.js
scripts only exist under the allowed persona plugins, creating compliance and runtime-governance
risk.
Agent Prompt 
## Issue description
`plugins/connectors/testssl-inspector/scripts/scan.js` is a Node.js runtime entrypoint, but it lives under `plugins/connectors/...`, which is not one of the allowed persona plugins.

## Issue Context
Compliance policy restricts Node.js scripts to persona plugins named exactly: `grc-engineer`, `grc-auditor`, `grc-internal`, or `grc-tprm`. This PR modifies a Node.js script in a connector plugin, which violates that restriction.

## Fix Focus Areas
- plugins/connectors/testssl-inspector/scripts/scan.js[35-46]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


async function main(argv) {
const args = parseArgs(argv);
const log = args.quiet ? () => {} : (m) => process.stderr.write(`[${SOURCE}] ${m}\n`);
Expand All @@ -47,7 +60,7 @@ async function main(argv) {

const mode = args.fast ? 'fast' : 'full';
const useDocker = args.docker ?? (config.use_docker === true);
const runner = await resolveRunner({ useDocker, configBinary: config.testssl_path });
const runner = await resolveRunner({ useDocker, configBinary: config.testssl_path, starttls: args.starttls });
log(`runner=${runner.label} mode=${mode} targets=${targets.length}`);

// Pre-fetch SCF crosswalks for the frameworks we expand to. Done once per run.
Expand All @@ -73,8 +86,9 @@ async function main(argv) {

for (const target of targets) {
try {
const raw = await runTestssl(runner, target, mode, log);
const doc = normalizeTargetFindings(raw, target, runId, runner.version, expansion);
const finalTarget = withDefaultStarttlsPort(target, args.starttls);
const raw = await runTestssl(runner, finalTarget, mode, args.starttls, log);
const doc = normalizeTargetFindings(raw, finalTarget, runId, runner.version, expansion);
findings.push(doc);
} catch (err) {
errors.push({ target, error: err.message });
Expand Down Expand Up @@ -121,7 +135,7 @@ async function main(argv) {
}

function parseArgs(argv) {
const args = { targets: [], fast: false, docker: undefined, quiet: false, output: 'summary', scfOnly: false, offline: false };
const args = { targets: [], fast: false, docker: undefined, quiet: false, output: 'summary', scfOnly: false, offline: false, starttls: null };
for (const a of argv) {
if (a === '--fast') args.fast = true;
else if (a === '--full') args.fast = false;
Expand All @@ -132,9 +146,21 @@ function parseArgs(argv) {
else if (a === '--offline') args.offline = true;
else if (a.startsWith('--target=')) args.targets.push(a.slice(9));
else if (a.startsWith('--output=')) args.output = a.slice(9);
else if (a.startsWith('--starttls=')) {
const proto = a.slice('--starttls='.length).toLowerCase();

if (!Object.prototype.hasOwnProperty.call(STARTTLS_PORTS, proto)) {
fail(
EXIT.USAGE,
`unknown STARTTLS protocol: ${proto}. Supported protocols: ${SUPPORTED_STARTTLS}`
);
}

args.starttls = proto;
}
else if (a === '-h' || a === '--help') {
process.stdout.write(
`Usage: scan.js --target=host[:port] [--target=...] [--fast|--full] [--docker] [--scf-only] [--offline] [--output=summary|silent|json] [--quiet]\n`
`Usage: scan.js --target=host[:port] [--target=...] [--starttls=protocol] [--fast|--full] [--docker] [--scf-only] [--offline] [--output=summary|silent|json] [--quiet]\n`
);
process.exit(0);
} else if (!a.startsWith('-')) args.targets.push(a);
Expand All @@ -144,7 +170,7 @@ function parseArgs(argv) {
}

/** Resolve which testssl invocation we'll use. */
async function resolveRunner({ useDocker, configBinary }) {
async function resolveRunner({ useDocker, configBinary, starttls }) {
if (useDocker) {
if (!await commandExists('docker')) fail(EXIT.TOOL_UNAVAILABLE, 'docker not on PATH — drop --docker or install Docker.');
return {
Expand All @@ -154,7 +180,7 @@ async function resolveRunner({ useDocker, configBinary }) {
'run', '--rm', '--network', 'host',
'-v', `${CACHE_DIR}:/tmp/scan-out`,
'drwetter/testssl.sh:latest',
...testsslArgs(target, mode, '/tmp/scan-out'),
...testsslArgs(target, mode, starttls, '/tmp/scan-out'),
],
cmd: 'docker',
};
Expand All @@ -171,7 +197,7 @@ async function resolveRunner({ useDocker, configBinary }) {
label: binary,
version,
cmd: binary,
argv: (target, mode) => testsslArgs(target, mode),
argv: (target, mode) => testsslArgs(target, mode, starttls),
};
}

Expand Down Expand Up @@ -201,20 +227,30 @@ function parseTestsslVersion(text) {
return m ? m[1] : 'unknown';
}

function testsslArgs(target, mode, outDir = null) {
function withDefaultStarttlsPort(target, starttls) {
if (!starttls) return target;
if (target.includes(':')) return target;

return `${target}:${STARTTLS_PORTS[starttls]}`;
Comment on lines +230 to +234

@coderabbitai coderabbitai Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix explicit-port detection for IPv6 targets

target.includes(':') treats IPv6 host literals as “has explicit port”, so --starttls=<proto> won’t append the default port for targets like [2001:db8::1]. That breaks the “append default port when port is omitted” behavior.

Suggested patch 
 function withDefaultStarttlsPort(target, starttls) {
   if (!starttls) return target;
-  if (target.includes(':')) return target;
+  const hasExplicitPort = target.startsWith('[')
+    ? /^\[[^\]]+\]:\d+$/.test(target)   // bracketed IPv6 with :port
+    : /:\d+$/.test(target);             // hostname/IPv4 with :port
+  if (hasExplicitPort) return target;
 
   return `${target}:${STARTTLS_PORTS[starttls]}`;
 }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/connectors/testssl-inspector/scripts/scan.js` around lines 230 - 234,
The withDefaultStarttlsPort function incorrectly detects explicit ports by
checking if target.includes(':'), which treats IPv6 addresses as having explicit
ports even when they don't (e.g., [2001:db8::1] has colons but no port). Fix the
port detection logic to properly handle IPv6 literals in square brackets: if the
target starts with '[', check whether there's a colon after the closing bracket
']' to determine if a port is specified; otherwise, check for the presence of a
colon to determine if a port exists. This ensures the default port is correctly
appended only when no explicit port is provided.

}

function testsslArgs(target, mode, starttls = null, outDir = null) {
const out = outDir || CACHE_DIR;
const jsonPath = path.join(out, `testssl-raw-${Date.now()}-${Math.random().toString(36).slice(2, 8)}.json`);
const base = [
'--quiet', '--warnings', 'off', '--color', '0',
'--jsonfile-pretty', jsonPath,
];
if (mode === 'fast') base.push('--fast');
if (starttls) {
base.push('--starttls', starttls);
}
base.push(target);
base.__jsonPath = jsonPath;
return base;
}

async function runTestssl(runner, target, mode, log) {
async function runTestssl(runner, target, mode, starttls, log) {

@greptile-apps greptile-apps Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The starttls parameter added to runTestssl is never read inside the function body. The --starttls flag is already baked into the runner.argv closure built by resolveRunner, so the argument has no effect here. The dead parameter may mislead future maintainers into thinking they can call runTestssl with a different starttls value and get a different result.

Suggested change
async function runTestssl(runner, target, mode, starttls, log) {
async function runTestssl(runner, target, mode, log) {

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Fix in Claude Code Fix in Codex Fix in Cursor

const argv = runner.argv(target, mode);
const jsonPath = argv.__jsonPath;
log(`scanning ${target}`);
Comment on lines 238 to 256

@qodo-code-review qodo-code-review Bot Jun 15, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Docker json path mismatch 🐞 Bug ☼ Reliability

In Docker mode the JSON output path is generated as a container path (/tmp/scan-out/...), but
runTestssl() later reads that same path on the host filesystem, so --docker scans will fail when
trying to load the JSON results.
Agent Prompt 
### Issue description
In `--docker` mode, `testsslArgs()` sets `argv.__jsonPath` to a container path (e.g. `/tmp/scan-out/...json`). After `docker run` completes, `runTestssl()` uses `fs.readFile(jsonPath)` on the host, which should instead read from `${CACHE_DIR}/...json` (the bind-mounted directory).

### Issue Context
Docker binds `${CACHE_DIR}` (host) to `/tmp/scan-out` (container). The JSON file is written inside the container to `/tmp/scan-out/<file>`, which appears on the host at `${CACHE_DIR}/<file>`.

### Fix Focus Areas
- plugins/connectors/testssl-inspector/scripts/scan.js[173-201]
- plugins/connectors/testssl-inspector/scripts/scan.js[237-263]

### Suggested approach
- Generate a filename once (e.g. `const fname = ...`), and set:
  - `containerJsonPath = path.posix.join('/tmp/scan-out', fname)` for the `--jsonfile-pretty` argument.
  - `hostJsonPath = path.join(CACHE_DIR, fname)` for `base.__jsonPath`.
- Alternatively, keep `--jsonfile-pretty` pointing at the container path but translate `argv.__jsonPath` to the host path in the docker runner (e.g., replace the `/tmp/scan-out/` prefix with `${CACHE_DIR}/` before reading).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Expand Down
Loading