Skip to content

fix(mitm): add Linux cert install and skip sudo password when root#1999

Merged
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.0from
NekoMonci12:main
May 7, 2026
Merged

fix(mitm): add Linux cert install and skip sudo password when root#1999
diegosouzapw merged 2 commits into
diegosouzapw:release/v3.8.0from
NekoMonci12:main

Conversation

@NekoMonci12
Copy link
Copy Markdown
Contributor

@NekoMonci12 NekoMonci12 commented May 6, 2026

Summary

Add Linux certificate management via update-ca-certificates for Docker support. Skip sudo password validation when running as root, matching the existing cli-tools route behavior.

Turbopack resolveAlias (@/mitm/manager → manager.stub.ts) was designed
for build-time safety but Next.js applies aliases to ALL imports
including dynamic ones. This caused await import("@/mitm/manager") at
runtime to load the stub, which silently returned fake {running: true}
without spawning the MITM proxy. The UI showed "MITM proxy started"
but nothing was actually running.

Fix introduces a two-path design:

  • @/mitm/manager → stub (build-time, safe for Turbopack)
  • @/mitm/manager.runtime → real manager (runtime, bypasses alias)

Route handlers now dynamic-import from manager.runtime, which
re-exports from ./manager and does NOT match the alias pattern.

Additional fixes:

  • Make stub throw explicit errors at runtime so misconfiguration is
    immediately visible instead of silently faking success
  • Add server.cjs to outputFileTracingIncludes (NFT trace) and Dockerfile
    COPY so the MITM server binary exists in standalone/Docker output

Related Issues

  • Closes #
  • Related to #

Validation

  • npm run lint
  • npm run test:unit
  • npm run test:coverage
  • Coverage is still >= 60% for statements, lines, functions, and branches
  • SonarQube PR analysis is green or any remaining issues are explicitly documented below

Tests Added Or Updated

  • List every changed or added automated test file.
  • If no production code changed, state that here.

Coverage Notes

  • If this PR changes src/, open-sse/, electron/, or bin/, explain which tests cover the change.
  • If coverage moved down in any touched file, explain why and what follow-up task will recover it.

Reviewer Notes

  • Call out any risky areas, migrations, feature flags, or manual validation that reviewers should know about.

Add Linux certificate management via update-ca-certificates for Docker support. Skip sudo password validation when running as root, matching the existing cli-tools route behavior.
@NekoMonci12 NekoMonci12 requested a review from diegosouzapw as a code owner May 6, 2026 13:18
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces Linux support for MITM certificate management and allows root users on non-Windows platforms to bypass the sudo password requirement. The review feedback suggests optimizing the Linux certificate installation process by combining multiple sudo commands into a single shell execution to reduce password prompts. Additionally, it recommends refining error handling to be platform-appropriate, avoiding the aggressive --fresh flag during certificate updates to prevent side effects, and ensuring original error messages are preserved for better debugging.

Comment thread src/mitm/cert/install.ts
Comment on lines +123 to +125
await execFileWithPassword("sudo", ["-S", "mkdir", "-p", LINUX_CA_DIR], sudoPassword);
await execFileWithPassword("sudo", ["-S", "cp", certPath, LINUX_CERT_DEST], sudoPassword);
await execFileWithPassword("sudo", ["-S", "update-ca-certificates"], sudoPassword);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Executing multiple sequential sudo commands is inefficient and will result in multiple password prompts if the user is not running as root and sudo is configured to require a password for each invocation. It is better to combine these into a single command string executed via sh -c to ensure only one authorization is needed.

Suggested change
await execFileWithPassword("sudo", ["-S", "mkdir", "-p", LINUX_CA_DIR], sudoPassword);
await execFileWithPassword("sudo", ["-S", "cp", certPath, LINUX_CERT_DEST], sudoPassword);
await execFileWithPassword("sudo", ["-S", "update-ca-certificates"], sudoPassword);
const command = `mkdir -p "${LINUX_CA_DIR}" && cp "${certPath}" "${LINUX_CERT_DEST}" && update-ca-certificates`;
await execFileWithPassword("sudo", ["-S", "sh", "-c", command], sudoPassword);

Comment thread src/mitm/cert/install.ts
Comment on lines +128 to +130
const msg = message.includes("canceled")
? "User canceled authorization"
: "Certificate install failed";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The check for the string "canceled" in the error message is specific to macOS security tools and is unlikely to be triggered by standard Linux sudo or system commands. On Linux, failures are more likely to be due to incorrect passwords or permission issues. Consider refining the error handling to be platform-appropriate or more generic.

Comment thread src/mitm/cert/install.ts
if (fs.existsSync(LINUX_CERT_DEST)) {
await execFileWithPassword("sudo", ["-S", "rm", "-f", LINUX_CERT_DEST], sudoPassword);
}
await execFileWithPassword("sudo", ["-S", "update-ca-certificates", "--fresh"], sudoPassword);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The --fresh flag for update-ca-certificates is quite aggressive as it re-generates all symlinks in /etc/ssl/certs and removes any that are broken. While it ensures the removed certificate is fully purged, it may have unintended side effects on other manually managed certificates in the system store. A standard update-ca-certificates call is usually sufficient after deleting the source file.

Suggested change
await execFileWithPassword("sudo", ["-S", "update-ca-certificates", "--fresh"], sudoPassword);
await execFileWithPassword("sudo", ["-S", "update-ca-certificates"], sudoPassword);

Comment thread src/mitm/cert/install.ts
}
await execFileWithPassword("sudo", ["-S", "update-ca-certificates", "--fresh"], sudoPassword);
} catch (err) {
throw new Error("Failed to uninstall certificate");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

The actual error message is being swallowed here, which makes debugging difficult if the uninstallation fails. It is better to include the original error message in the thrown Error.

Suggested change
throw new Error("Failed to uninstall certificate");
throw new Error(`Failed to uninstall certificate: ${getErrorMessage(err)}`);

@NekoMonci12 NekoMonci12 marked this pull request as draft May 6, 2026 13:39
Turbopack resolveAlias (@/mitm/manager → manager.stub.ts) was designed
for build-time safety but Next.js applies aliases to ALL imports —
including dynamic ones. This caused await import("@/mitm/manager") at
runtime to load the stub, which silently returned fake {running: true}
without spawning the MITM proxy. The UI showed "MITM proxy started"
but nothing was actually running.

Fix introduces a two-path design:
- @/mitm/manager        → stub (build-time, safe for Turbopack)
- @/mitm/manager.runtime → real manager (runtime, bypasses alias)

Route handlers now dynamic-import from manager.runtime, which
re-exports from ./manager and does NOT match the alias pattern.

Additional fixes:
- Make stub throw explicit errors at runtime so misconfiguration is
  immediately visible instead of silently faking success
- Add server.cjs to outputFileTracingIncludes (NFT trace) and Dockerfile
  COPY so the MITM server binary exists in standalone/Docker output
@NekoMonci12 NekoMonci12 marked this pull request as ready for review May 7, 2026 03:38
@diegosouzapw diegosouzapw changed the base branch from main to release/v3.8.0 May 7, 2026 11:24
@diegosouzapw
Copy link
Copy Markdown
Owner

Thank you @NekoMonci12 for your contribution! This has been reviewed and is now merged into release/v3.8.0 for the upcoming release. Amazing work!

@diegosouzapw diegosouzapw merged commit 72d0e1f into diegosouzapw:release/v3.8.0 May 7, 2026
67 of 123 checks passed
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.

2 participants