Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add save-state and set-output file commands #1178

Merged
merged 3 commits into from Sep 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
132 changes: 125 additions & 7 deletions packages/core/__tests__/core.test.ts
Expand Up @@ -41,7 +41,9 @@ const testEnvVars = {

// File Commands
GITHUB_PATH: '',
GITHUB_ENV: ''
GITHUB_ENV: '',
GITHUB_OUTPUT: '',
GITHUB_STATE: ''
}

const UUID = '9b1deb4d-3b7d-4bad-9bdd-2b0d7b3dcb6d'
Expand Down Expand Up @@ -283,24 +285,82 @@ describe('@actions/core', () => {
).toEqual([' val1 ', ' val2 ', ' '])
})

it('setOutput produces the correct command', () => {
it('legacy setOutput produces the correct command', () => {
core.setOutput('some output', 'some value')
assertWriteCalls([
os.EOL,
`::set-output name=some output::some value${os.EOL}`
])
})

it('setOutput handles bools', () => {
it('legacy setOutput handles bools', () => {
core.setOutput('some output', false)
assertWriteCalls([os.EOL, `::set-output name=some output::false${os.EOL}`])
})

it('setOutput handles numbers', () => {
it('legacy setOutput handles numbers', () => {
core.setOutput('some output', 1.01)
assertWriteCalls([os.EOL, `::set-output name=some output::1.01${os.EOL}`])
})

it('setOutput produces the correct command and sets the output', () => {
const command = 'OUTPUT'
createFileCommandFile(command)
core.setOutput('my out', 'out val')
verifyFileCommand(
command,
`my out<<${DELIMITER}${os.EOL}out val${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('setOutput handles boolean inputs', () => {
const command = 'OUTPUT'
createFileCommandFile(command)
core.setOutput('my out', true)
verifyFileCommand(
command,
`my out<<${DELIMITER}${os.EOL}true${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('setOutput handles number inputs', () => {
const command = 'OUTPUT'
createFileCommandFile(command)
core.setOutput('my out', 5)
verifyFileCommand(
command,
`my out<<${DELIMITER}${os.EOL}5${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('setOutput does not allow delimiter as value', () => {
const command = 'OUTPUT'
createFileCommandFile(command)

expect(() => {
core.setOutput('my out', `good stuff ${DELIMITER} bad stuff`)
}).toThrow(
`Unexpected input: value should not contain the delimiter "${DELIMITER}"`
)

const filePath = path.join(__dirname, `test/${command}`)
fs.unlinkSync(filePath)
})

it('setOutput does not allow delimiter as name', () => {
const command = 'OUTPUT'
createFileCommandFile(command)

expect(() => {
core.setOutput(`good stuff ${DELIMITER} bad stuff`, 'test')
}).toThrow(
`Unexpected input: name should not contain the delimiter "${DELIMITER}"`
)

const filePath = path.join(__dirname, `test/${command}`)
fs.unlinkSync(filePath)
})

it('setFailed sets the correct exit code and failure message', () => {
core.setFailed('Failure message')
expect(process.exitCode).toBe(core.ExitCode.Failure)
Expand Down Expand Up @@ -466,21 +526,79 @@ describe('@actions/core', () => {
assertWriteCalls([`::debug::%0D%0Adebug%0A${os.EOL}`])
})

it('saveState produces the correct command', () => {
it('legacy saveState produces the correct command', () => {
core.saveState('state_1', 'some value')
assertWriteCalls([`::save-state name=state_1::some value${os.EOL}`])
})

it('saveState handles numbers', () => {
it('legacy saveState handles numbers', () => {
core.saveState('state_1', 1)
assertWriteCalls([`::save-state name=state_1::1${os.EOL}`])
})

it('saveState handles bools', () => {
it('legacy saveState handles bools', () => {
core.saveState('state_1', true)
assertWriteCalls([`::save-state name=state_1::true${os.EOL}`])
})

it('saveState produces the correct command and saves the state', () => {
const command = 'STATE'
createFileCommandFile(command)
core.saveState('my state', 'out val')
verifyFileCommand(
command,
`my state<<${DELIMITER}${os.EOL}out val${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('saveState handles boolean inputs', () => {
const command = 'STATE'
createFileCommandFile(command)
core.saveState('my state', true)
verifyFileCommand(
command,
`my state<<${DELIMITER}${os.EOL}true${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('saveState handles number inputs', () => {
const command = 'STATE'
createFileCommandFile(command)
core.saveState('my state', 5)
verifyFileCommand(
command,
`my state<<${DELIMITER}${os.EOL}5${os.EOL}${DELIMITER}${os.EOL}`
)
})

it('saveState does not allow delimiter as value', () => {
const command = 'STATE'
createFileCommandFile(command)

expect(() => {
core.saveState('my state', `good stuff ${DELIMITER} bad stuff`)
}).toThrow(
`Unexpected input: value should not contain the delimiter "${DELIMITER}"`
)

const filePath = path.join(__dirname, `test/${command}`)
fs.unlinkSync(filePath)
})

it('saveState does not allow delimiter as name', () => {
const command = 'STATE'
createFileCommandFile(command)

expect(() => {
core.saveState(`good stuff ${DELIMITER} bad stuff`, 'test')
}).toThrow(
`Unexpected input: name should not contain the delimiter "${DELIMITER}"`
)

const filePath = path.join(__dirname, `test/${command}`)
fs.unlinkSync(filePath)
})

it('getState gets wrapper action state', () => {
expect(core.getState('TEST_1')).toBe('state_val')
})
Expand Down
39 changes: 16 additions & 23 deletions packages/core/src/core.ts
@@ -1,10 +1,9 @@
import {issue, issueCommand} from './command'
import {issueCommand as issueFileCommand} from './file-command'
import {issueFileCommand, prepareKeyValueMessage} from './file-command'
import {toCommandProperties, toCommandValue} from './utils'

import * as os from 'os'
import * as path from 'path'
import {v4 as uuidv4} from 'uuid'

import {OidcClient} from './oidc-utils'

Expand Down Expand Up @@ -87,26 +86,10 @@ export function exportVariable(name: string, val: any): void {

const filePath = process.env['GITHUB_ENV'] || ''
if (filePath) {
const delimiter = `ghadelimiter_${uuidv4()}`

// These should realistically never happen, but just in case someone finds a way to exploit uuid generation let's not allow keys or values that contain the delimiter.
if (name.includes(delimiter)) {
throw new Error(
`Unexpected input: name should not contain the delimiter "${delimiter}"`
)
}

if (convertedVal.includes(delimiter)) {
throw new Error(
`Unexpected input: value should not contain the delimiter "${delimiter}"`
)
}

const commandValue = `${name}<<${delimiter}${os.EOL}${convertedVal}${os.EOL}${delimiter}`
issueFileCommand('ENV', commandValue)
} else {
issueCommand('set-env', {name}, convertedVal)
return issueFileCommand('ENV', prepareKeyValueMessage(name, val))
}

issueCommand('set-env', {name}, convertedVal)
}

/**
Expand Down Expand Up @@ -207,8 +190,13 @@ export function getBooleanInput(name: string, options?: InputOptions): boolean {
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function setOutput(name: string, value: any): void {
const filePath = process.env['GITHUB_OUTPUT'] || ''
if (filePath) {
return issueFileCommand('OUTPUT', prepareKeyValueMessage(name, value))
}

process.stdout.write(os.EOL)
issueCommand('set-output', {name}, value)
issueCommand('set-output', {name}, toCommandValue(value))
}

/**
Expand Down Expand Up @@ -362,7 +350,12 @@ export async function group<T>(name: string, fn: () => Promise<T>): Promise<T> {
*/
// eslint-disable-next-line @typescript-eslint/no-explicit-any
export function saveState(name: string, value: any): void {
issueCommand('save-state', {name}, value)
const filePath = process.env['GITHUB_STATE'] || ''
if (filePath) {
return issueFileCommand('STATE', prepareKeyValueMessage(name, value))
}

issueCommand('save-state', {name}, toCommandValue(value))
}

/**
Expand Down
25 changes: 24 additions & 1 deletion packages/core/src/file-command.ts
Expand Up @@ -5,9 +5,10 @@

import * as fs from 'fs'
import * as os from 'os'
import {v4 as uuidv4} from 'uuid'
import {toCommandValue} from './utils'

export function issueCommand(command: string, message: any): void {
export function issueFileCommand(command: string, message: any): void {
const filePath = process.env[`GITHUB_${command}`]
if (!filePath) {
throw new Error(
Expand All @@ -22,3 +23,25 @@ export function issueCommand(command: string, message: any): void {
encoding: 'utf8'
})
}

export function prepareKeyValueMessage(key: string, value: any): string {
const delimiter = `ghadelimiter_${uuidv4()}`
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 on the refactor

const convertedValue = toCommandValue(value)

// These should realistically never happen, but just in case someone finds a
// way to exploit uuid generation let's not allow keys or values that contain
// the delimiter.
if (key.includes(delimiter)) {
throw new Error(
`Unexpected input: name should not contain the delimiter "${delimiter}"`
)
}
Comment on lines +34 to +38
Copy link

Choose a reason for hiding this comment

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

The check for the delimiter in the key feels a little misleading to me - if the key is user-controlled, it can still be used to add additional outputs if that's the risk being mitigated here; the delimiter doesn't capture the key, it just captures the value.

Suggested change
if (key.includes(delimiter)) {
throw new Error(
`Unexpected input: name should not contain the delimiter "${delimiter}"`
)
}


if (convertedValue.includes(delimiter)) {
throw new Error(
`Unexpected input: value should not contain the delimiter "${delimiter}"`
)
}

return `${key}<<${delimiter}${os.EOL}${convertedValue}${os.EOL}${delimiter}`
}