diff --git a/CHANGELOG.md b/CHANGELOG.md index 475e6c772..642a5c878 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,7 @@ ## {{ UNRELEASED_VERSION }} - [{{ UNRELEASED_DATE }}]({{ UNRELEASED_LINK }}) +* Fixed ANSI escape codes appearing in redirected output by checking `stdout.isTTY` instead of `stdin.isTTY` for TTY allocation [#345](https://github.com/lando/core/issues/345) + ## v3.26.2 - [December 17, 2025](https://github.com/lando/core/releases/tag/v3.26.2) * Updated to use new Lando Alliance Apple Developer certificates diff --git a/lib/compose.js b/lib/compose.js index 34cc67c18..2a5721cfe 100644 --- a/lib/compose.js +++ b/lib/compose.js @@ -26,7 +26,7 @@ const composeFlags = { const defaultOptions = { build: {noCache: false, pull: true}, down: {removeOrphans: true, volumes: true}, - exec: {detach: false, noTTY: !process.stdin.isTTY}, + exec: {detach: false, noTTY: !(process.stdin.isTTY && process.stdout.isTTY)}, kill: {}, logs: {follow: false, timestamps: false}, ps: {q: true}, diff --git a/test/tty-allocation.spec.js b/test/tty-allocation.spec.js new file mode 100644 index 000000000..64edab6b2 --- /dev/null +++ b/test/tty-allocation.spec.js @@ -0,0 +1,147 @@ +/* + * Tests for TTY allocation in docker exec and compose. + * @file tty-allocation.spec.js + * + * Validates that TTY is only allocated when BOTH stdin and stdout + * are TTYs. When stdout is redirected (e.g. `lando foo > file.txt`), + * TTY should NOT be allocated so that ANSI escape codes are not + * written to the file. + * + * @see https://github.com/lando/core/issues/345 + * @see https://github.com/lando/drupal/issues/157 + */ + +'use strict'; + +const chai = require('chai'); +const expect = chai.expect; +chai.should(); + +describe('TTY allocation', () => { + // Save originals + const originalStdinIsTTY = process.stdin.isTTY; + const originalStdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + // Clear require cache so compose.js re-evaluates with test-set TTY values + delete require.cache[require.resolve('./../lib/compose')]; + delete require.cache[require.resolve('./../utils/build-docker-exec')]; + }); + + afterEach(() => { + // Restore after each test + process.stdin.isTTY = originalStdinIsTTY; + process.stdout.isTTY = originalStdoutIsTTY; + // Clear require cache so compose.js re-evaluates + delete require.cache[require.resolve('./../lib/compose')]; + delete require.cache[require.resolve('./../utils/build-docker-exec')]; + }); + + describe('compose exec (lib/compose.js)', () => { + it('should set noTTY=true when stdout is not a TTY (output redirected)', () => { + process.stdin.isTTY = true; + process.stdout.isTTY = false; + const compose = require('./../lib/compose'); + const result = compose.run( + ['docker-compose.yml'], + 'test_project', + {services: ['web'], cmd: ['echo', 'hello']}, + ); + // When noTTY is true, the -T flag should be in the command + expect(result.cmd).to.include('-T'); + }); + + it('should set noTTY=false when both stdin and stdout are TTYs', () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + const compose = require('./../lib/compose'); + const result = compose.run( + ['docker-compose.yml'], + 'test_project', + {services: ['web'], cmd: ['echo', 'hello']}, + ); + // When both are TTY, -T should NOT be in the command + expect(result.cmd).to.not.include('-T'); + }); + + it('should set noTTY=true when stdin is not a TTY (non-interactive)', () => { + process.stdin.isTTY = false; + process.stdout.isTTY = true; + const compose = require('./../lib/compose'); + const result = compose.run( + ['docker-compose.yml'], + 'test_project', + {services: ['web'], cmd: ['echo', 'hello']}, + ); + expect(result.cmd).to.include('-T'); + }); + + it('should set noTTY=true when neither stdin nor stdout is a TTY', () => { + process.stdin.isTTY = false; + process.stdout.isTTY = false; + const compose = require('./../lib/compose'); + const result = compose.run( + ['docker-compose.yml'], + 'test_project', + {services: ['web'], cmd: ['echo', 'hello']}, + ); + expect(result.cmd).to.include('-T'); + }); + }); + + describe('docker exec (utils/build-docker-exec.js)', () => { + it('should not include --tty when stdout is not a TTY', () => { + process.stdin.isTTY = true; + process.stdout.isTTY = false; + const buildDockerExec = require('./../utils/build-docker-exec'); + + let capturedCmd; + const injected = { + config: {dockerBin: 'docker'}, + _config: {dockerBin: 'docker'}, + shell: { + sh: (cmd, opts) => { + capturedCmd = cmd; + return Promise.resolve(); + }, + }, + }; + + const datum = { + id: 'test_container', + cmd: ['echo', 'hello'], + opts: {user: 'www-data', environment: {}}, + }; + + buildDockerExec(injected, 'inherit', datum); + expect(capturedCmd).to.not.include('--tty'); + }); + + it('should include --tty when both stdin and stdout are TTYs', () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + const buildDockerExec = require('./../utils/build-docker-exec'); + + let capturedCmd; + const injected = { + config: {dockerBin: 'docker'}, + _config: {dockerBin: 'docker'}, + shell: { + sh: (cmd, opts) => { + capturedCmd = cmd; + return Promise.resolve(); + }, + }, + }; + + const datum = { + id: 'test_container', + cmd: ['echo', 'hello'], + opts: {user: 'www-data', environment: {}}, + }; + + buildDockerExec(injected, 'inherit', datum); + expect(capturedCmd).to.include('--tty'); + }); + }); +}); diff --git a/utils/build-docker-exec.js b/utils/build-docker-exec.js index 651f505e2..eddb6c99e 100644 --- a/utils/build-docker-exec.js +++ b/utils/build-docker-exec.js @@ -8,7 +8,7 @@ const _ = require('lodash'); const getExecOpts = (docker, datum) => { const exec = [docker, 'exec']; // Should only use this if we have to - if (process.stdin.isTTY) exec.push('--tty'); + if (process.stdin.isTTY && process.stdout.isTTY) exec.push('--tty'); // Should only set interactive in node mode if (process.lando === 'node') exec.push('--interactive'); // add workdir if we can