From 27a4470519baedea4a547856638e19edd3cd92d8 Mon Sep 17 00:00:00 2001 From: Laurence de Bruxelles Date: Thu, 8 Dec 2022 17:01:38 +0000 Subject: [PATCH] Stop user home path from being printed in migration script log --- CHANGELOG.md | 1 + __tests__/spec/migrate.js | 22 ++++++++++++++++++++-- lib/migrator/fileHelpers.js | 15 ++++++++++----- lib/migrator/logger.js | 11 +++++++++-- lib/migrator/logger.test.js | 20 ++++++++++++++++++++ 5 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 lib/migrator/logger.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c6f166df4..2b06d54c99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- [Stop user's home path from being printed in migration script log](https://github.com/alphagov/govuk-prototype-kit/pull/1847) - [Fix crashes when path to prototype contains spaces](https://github.com/alphagov/govuk-prototype-kit/pull/1841) ## 13.0.1 diff --git a/__tests__/spec/migrate.js b/__tests__/spec/migrate.js index a957273f84..0a15f1f8d6 100644 --- a/__tests__/spec/migrate.js +++ b/__tests__/spec/migrate.js @@ -1,12 +1,16 @@ /* eslint-env jest */ -const path = require('path') const fs = require('fs') +const os = require('os') +const path = require('path') + const fse = require('fs-extra') + const { spawn } = require('../../lib/exec') const { mkdtempSync } = require('../util') -const projectDirectory = path.join(mkdtempSync(), 'migrate-checks') +const testDirectory = mkdtempSync() +const projectDirectory = path.join(testDirectory, 'migrate-checks') const appDirectory = path.join(projectDirectory, 'app') const assetsDirectory = path.join(appDirectory, 'assets') const fixtureProjectDirectory = path.join(__dirname, '..', 'fixtures', 'test-v11-prototype') @@ -137,4 +141,18 @@ describe('migrate test prototype', () => { '{% extends "govuk-prototype-kit/layouts/govuk-branded.html" %}' + '\n' ) }) + + it('migrate.log does not contain user home directory', () => { + const migrateLog = fs.readFileSync(path.join(projectDirectory, 'migrate.log'), 'utf8') + + expect(migrateLog).not.toContain(os.homedir()) + + // Because we don't run the test in the home directory, we also have to do + // a slightly different check, we make sure all paths except for the + // metadata in the header lines are relative to the prototype directory. + // If this is true then this should mean we won't get the home dir path. + expect(migrateLog.split('\n').filter( + line => line.includes(testDirectory) && !line.startsWith('argv:') && !line.startsWith('cwd:') + )).toEqual([]) + }) }) diff --git a/lib/migrator/fileHelpers.js b/lib/migrator/fileHelpers.js index 8b1eae113d..2c245fb48f 100644 --- a/lib/migrator/fileHelpers.js +++ b/lib/migrator/fileHelpers.js @@ -1,8 +1,10 @@ const path = require('path') -const { packageDir, projectDir } = require('../path-utils') const fs = require('fs').promises + const fse = require('fs-extra') -const { log } = require('./logger') + +const { packageDir, projectDir } = require('../path-utils') +const { log, sanitisePaths } = require('./logger') const verboseLog = async function () { await log(...arguments) @@ -64,15 +66,16 @@ const replaceStartOfFile = async ({ filePath, lineToFind, replacement }) => { } const removeLineFromFile = async ({ filePath, lineToRemove }) => { + const prettyFilePath = path.relative(projectDir, filePath) const linesToRemove = Array.isArray(lineToRemove) ? lineToRemove : [lineToRemove] const fileLines = await getFileAsLines(filePath) await verboseLog(fileLines) const updatedFileLines = fileLines.filter(line => !linesToRemove.includes(line)) if (updatedFileLines.length !== fileLines.length) { - await verboseLog(`Found [${lineToRemove}] in [${filePath}, removing`) + await verboseLog(`Found [${lineToRemove}] in [${prettyFilePath}], removing`) return writeFileLinesToFile(filePath, updatedFileLines) } - await verboseLog(`Could not found [${lineToRemove}] in [${filePath}, aborting`) + await verboseLog(`Could not found [${lineToRemove}] in [${prettyFilePath}], aborting`) return false } @@ -102,7 +105,9 @@ const deleteDirectoryIfEmpty = async (partialPath) => { const copyFileFromStarter = async (starterPath, newPath) => { const src = path.join(packageDir, 'prototype-starter', starterPath) const dest = path.join(projectDir, newPath || starterPath) - await verboseLog(`Copying from [${path.relative(projectDir, src)}] to [${path.relative(projectDir, dest)}]`) + const prettySrc = src.startsWith(projectDir) ? path.relative(projectDir, src) : sanitisePaths(src) + const prettyDest = path.relative(projectDir, dest) + await verboseLog(`Copying from [${prettySrc}] to [${prettyDest}]`) return fse.copy(src, dest) .then(successOutput) .catch(async e => { diff --git a/lib/migrator/logger.js b/lib/migrator/logger.js index b92a8e5621..3ad1e5f97d 100644 --- a/lib/migrator/logger.js +++ b/lib/migrator/logger.js @@ -1,4 +1,6 @@ +const os = require('os') const path = require('path') + const { projectDir } = require('../path-utils') const fs = require('fs').promises @@ -7,15 +9,19 @@ const packageVersion = require('../../package.json').version const migrateLogFilePath = path.join(projectDir, 'migrate.log') let migrateLogFileHandle +function sanitisePaths (str) { + return str.replaceAll(os.homedir(), '~') +} + async function setup () { if (!migrateLogFileHandle) { migrateLogFileHandle = await fs.open(migrateLogFilePath, 'a') // log some information useful for debugging await module.exports.log(new Date().toISOString()) - await module.exports.log('cwd: ' + process.cwd()) + await module.exports.log('cwd: ' + sanitisePaths(process.cwd())) await module.exports.log(`package: govuk-prototype-kit@${packageVersion}`) - await module.exports.log('argv: ' + process.argv.join(' ')) + await module.exports.log('argv: ' + sanitisePaths(process.argv.join(' '))) } } @@ -31,6 +37,7 @@ async function log (message) { module.exports = { log, + sanitisePaths, setup, teardown } diff --git a/lib/migrator/logger.test.js b/lib/migrator/logger.test.js new file mode 100644 index 0000000000..e444ce02ca --- /dev/null +++ b/lib/migrator/logger.test.js @@ -0,0 +1,20 @@ +/* eslint-env jest */ + +describe('sanitisePaths', () => { + const os = require('os') + const path = require('path') + + const { sanitisePaths } = require('./logger') + + it('replaces occurences of home directory with ~', () => { + expect( + sanitisePaths(path.join(os.homedir(), 'path', 'to', 'folder')) + ).toEqual(path.join('~', 'path', 'to', 'folder')) + }) + + it('replaces all occurences in a string', () => { + expect( + sanitisePaths(process.argv.join(' ')) + ).not.toContain(os.homedir()) + }) +})