Merge pull request #1847 from alphagov/ldeb-redact-home-path

Stop user's home path from being printed in migration script log

CHANGELOG.md

@@ -4,7 +4,8 @@
 
 ### Fixes
 
-- [Fix crashes when path to prototype contains spaces](https://github.com/alphagov/govuk-prototype-kit/pull/1841)
+- [#1847: Stop user's home path from being printed in migration script log](https://github.com/alphagov/govuk-prototype-kit/pull/1847)
+- [#1841: Fix crashes when path to prototype contains spaces](https://github.com/alphagov/govuk-prototype-kit/pull/1841)
 
 ## 13.0.1
 

__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([])
+  })
 })

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)
@@ -49,29 +51,31 @@ const handleNotFound = resultIfFileNotFound => e => {
 }
 
 const replaceStartOfFile = async ({ filePath, lineToFind, replacement }) => {
+  const prettyFilePath = path.relative(projectDir, filePath)
   const fileLines = await getFileAsLines(filePath)
   const replacementLineNumber = fileLines.indexOf(lineToFind)
   if (replacementLineNumber > -1) {
-    await verboseLog(`Found [${lineToFind}] in [${filePath}, replacing`)
+    await verboseLog(`Found [${lineToFind}] in [${prettyFilePath}], replacing`)
     const linesFromOldFile = fileLines.splice(replacementLineNumber + 1)
     await verboseLog('Keeping these lines from old file', linesFromOldFile)
     const updatedFileLines = [replacement, ...linesFromOldFile]
     return writeFileLinesToFile(filePath, updatedFileLines)
   }
-  await verboseLog(`Could not found [${lineToFind}] in [${filePath}, aborting`)
+  await verboseLog(`Could not found [${lineToFind}] in [${prettyFilePath}], aborting`)
   return false
 }
 
 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
 }
 
@@ -101,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 [${src}] to [${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 => {

lib/migrator/index.js

@@ -58,9 +58,9 @@ const directoriesToDelete = [
 ]
 
 const directoriesToDeleteIfEmpty = [
-  '/app/assets/sass/patterns',
-  '/app/assets/images',
-  '/app/views/includes'
+  'app/assets/sass/patterns',
+  'app/assets/images',
+  'app/views/includes'
 ]
 
 const filesToUpdateIfUnchanged = [

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
 }

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())
+  })
+})

lib/migrator/migrationSteps.js

@@ -131,7 +131,7 @@ module.exports.deleteUnusedFiles = async (filesToDelete) => {
       // Do not report files that don't exist
       return true
     }
-    const reporter = await addReporter(`Delete ${filePath}`)
+    const reporter = await addReporter(`Delete ${file}`)
     const result = await deleteFile(path.join(projectDir, file))
     await reporter(result)
     return result