Skip to content

Commit

Permalink
Stop user home path from being printed in migration script log
Browse files Browse the repository at this point in the history
  • Loading branch information
lfdebrux committed Dec 8, 2022
1 parent 4129eb9 commit 27a4470
Show file tree
Hide file tree
Showing 5 changed files with 60 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
22 changes: 20 additions & 2 deletions __tests__/spec/migrate.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -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([])
})
})
15 changes: 10 additions & 5 deletions lib/migrator/fileHelpers.js
Original file line number Diff line number Diff line change
@@ -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)
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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 => {
Expand Down
11 changes: 9 additions & 2 deletions lib/migrator/logger.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
const os = require('os')
const path = require('path')

const { projectDir } = require('../path-utils')
const fs = require('fs').promises

Expand All @@ -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(' ')))
}
}

Expand All @@ -31,6 +37,7 @@ async function log (message) {

module.exports = {
log,
sanitisePaths,
setup,
teardown
}
20 changes: 20 additions & 0 deletions lib/migrator/logger.test.js
Original file line number Diff line number Diff line change
@@ -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())
})
})

0 comments on commit 27a4470

Please sign in to comment.