Skip to content

Commit

Permalink
fix: harden for option injection (#40)
Browse files Browse the repository at this point in the history
* feat: harden flagsAsArguments usage

* refactor: require whitelisting of flags

* test: fix

* build: update distributable
  • Loading branch information
sparten11740 authored Mar 13, 2024
1 parent b544594 commit adf651a
Show file tree
Hide file tree
Showing 6 changed files with 85 additions and 14 deletions.
12 changes: 9 additions & 3 deletions dist/version/index.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion dist/version/index.js.map

Large diffs are not rendered by default.

40 changes: 40 additions & 0 deletions src/utils/git.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import { add, commit, resetLastCommit } from './git'

describe('add', () => {
it('should throw when trying to use flags', () => {
expect(() => add(['--force', '.yarnrc.yml'])).toThrow('Options are not allowed')
expect(() => add(['--force', '.yarnrc.yml'])).toThrow('Options are not allowed')
})
})

describe('commit', () => {
it.each([
['verbose'],
['dryRun'],
['force'],
['patch'],
['sparse'],
['edit'],
['ignoreErrors'],
['interactive'],
['renormalize'],
['refresh'],
['ignoreMissing'],
['ignoreRemoval'],
])('should throw when using non-whitelisted %s flag', (flag) => {
expect(() =>
commit({ message: 'feat: taking all your crypto', flags: { [flag]: true } as never })
).toThrow('Only the following flags are allowed: amend, all, noEdit')
})
})

describe('resetLastCommit', () => {
it.each([['pathspecFileNul'], ['hard'], ['merge'], ['keep'], ['soft']])(
'should throw when using non-whitelisted %s flag',
(flag) => {
expect(() => resetLastCommit({ flags: { [flag]: true } })).toThrow(
'Only the following flags are allowed: mixed'
)
}
)
})
15 changes: 13 additions & 2 deletions src/utils/git.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import { spawnSync } from './process'
import { flagsAsArguments } from './objects'
import * as assert from 'node:assert'

export function add(pathSpecs: string[]) {
assert(
pathSpecs.every((it) => !it.startsWith('-')),
'Options are not allowed. Please supply paths to the files you want to add only.'
)

spawnSync('git', ['add', ...pathSpecs])
}

Expand All @@ -16,8 +22,11 @@ type CommitParams = {
body?: string
flags?: CommitFlags
}

const commitFlags = ['amend', 'all', 'noEdit']

export function commit({ message, body, flags }: CommitParams) {
const args = ['commit', ...flagsAsArguments(flags)]
const args = ['commit', ...flagsAsArguments(flags, commitFlags)]

if (message) {
args.push('-m', `"${message}"`)
Expand Down Expand Up @@ -88,8 +97,10 @@ type ResetLastCommitParams = {
flags: ResetFlags
}

const resetFlags = ['mixed']

export function resetLastCommit({ flags }: ResetLastCommitParams) {
spawnSync('git', ['reset', ...flagsAsArguments(flags), 'HEAD~1'])
spawnSync('git', ['reset', ...flagsAsArguments(flags, resetFlags), 'HEAD~1'])
}

type ConfigureUserParams = {
Expand Down
19 changes: 12 additions & 7 deletions src/utils/objects.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,24 @@ import { flagsAsArguments } from './objects'
describe('objects', () => {
describe('stringifyFlags', () => {
it('should stringify multiple flags', () => {
expect(flagsAsArguments({ ff: true, noEdit: false, forceWithLease: true })).toEqual([
'--ff',
'--force-with-lease',
])
expect(
flagsAsArguments({ ff: true, noEdit: false, forceWithLease: true }, [
'ff',
'noEdit',
'forceWithLease',
])
).toEqual(['--ff', '--force-with-lease'])
})

it('should stringify a single flag', () => {
expect(flagsAsArguments({ forceWithLease: true })).toEqual(['--force-with-lease'])
expect(flagsAsArguments({ forceWithLease: true }, ['forceWithLease'])).toEqual([
'--force-with-lease',
])
})

it('should stringify no flags', () => {
expect(flagsAsArguments({ forceWithLease: false })).toEqual([])
expect(flagsAsArguments({})).toEqual([])
expect(flagsAsArguments({ forceWithLease: false }, ['forceWithLease'])).toEqual([])
expect(flagsAsArguments({}, [])).toEqual([])
})
})
})
11 changes: 10 additions & 1 deletion src/utils/objects.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import { toKebabCase } from './strings'
import * as assert from 'node:assert'

export function flagsAsArguments(flags: { [key: string]: boolean } | undefined): string[] {
export function flagsAsArguments(
flags: { [key: string]: boolean } | undefined,
whitelistedFlags: string[]
): string[] {
return Object.entries(flags ?? {}).reduce<string[]>((all, [flag, enabled]) => {
assert(
whitelistedFlags.includes(flag),
`Only the following flags are allowed: ${whitelistedFlags.join(', ')}`
)

if (enabled) {
all.push(`--${toKebabCase(flag)}`)
}
Expand Down

0 comments on commit adf651a

Please sign in to comment.