daniel543x/craft-common-actions
1
0 Fork 0
mirror of https://github.com/storytold/craft-common-actions.git synced 2026-11-05 18:20:28 +00:00
Code Issues Packages Projects Releases Wiki Activity

security(release): prevent app-name shell injection in FreeBSD and server jobs (#55)

Browse Source 
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This commit is contained in:
coderabbitai[bot] authored and GitHub committed 2026-10-22 14:47:14 -04:00
1 parent 5140aec7d3
commit d6d11df79e
3 files changed
+146 -15
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

.github/workflows/release.yml
+18 -11
View File
@@ -635,6 +635,7 @@ jobs:
# takes over 5 hours for most apps. See storytold/craft-common-actions#39.
arch: [x86_64]
env:
APP_NAME: ${{ inputs.app-name }}
CARGO_HOME: /var/tmp/craft-cargo
CARGO_TARGET_DIR: /var/tmp/craft-target
CARGO_BUILD_JOBS: 4
@@ -703,7 +704,7 @@ jobs:
cpu: 4
mem: 12288
copyback: false
envs: "CARGO_HOME CARGO_TARGET_DIR CARGO_INCREMENTAL CARGO_BUILD_JOBS VERSION BUILD_DATE BUILD_SHA INPUT_VERSION CADCRAFT_VERSION CADCRAFT_BUILD_DATE CADCRAFT_BUILD_SHA ARTCRAFT_LAUNCHER_VERSION ARTCRAFT_LAUNCHER_BUILD_DATE ARTCRAFT_LAUNCHER_BUILD_SHA DECKCRAFT_VERSION DECKCRAFT_BUILD_DATE DECKCRAFT_BUILD_SHA DESIGNCRAFT_VERSION DESIGNCRAFT_BUILD_DATE DESIGNCRAFT_BUILD_SHA EFFECTCRAFT_VERSION EFFECTCRAFT_BUILD_DATE EFFECTCRAFT_BUILD_SHA FILMCRAFT_VERSION FILMCRAFT_BUILD_DATE FILMCRAFT_BUILD_SHA GRIDCRAFT_VERSION GRIDCRAFT_BUILD_DATE GRIDCRAFT_BUILD_SHA LIGHTCRAFT_VERSION LIGHTCRAFT_BUILD_DATE LIGHTCRAFT_BUILD_SHA PDFCRAFT_VERSION PDFCRAFT_BUILD_DATE PDFCRAFT_BUILD_SHA PHOTOCRAFT_VERSION PHOTOCRAFT_BUILD_DATE PHOTOCRAFT_BUILD_SHA SOUNDCRAFT_VERSION SOUNDCRAFT_BUILD_DATE SOUNDCRAFT_BUILD_SHA VECTORCRAFT_VERSION VECTORCRAFT_BUILD_DATE VECTORCRAFT_BUILD_SHA WORDCRAFT_VERSION WORDCRAFT_BUILD_DATE WORDCRAFT_BUILD_SHA FIGCRAFT_VERSION FIGCRAFT_BUILD_DATE FIGCRAFT_BUILD_SHA"
envs: "APP_NAME CARGO_HOME CARGO_TARGET_DIR CARGO_INCREMENTAL CARGO_BUILD_JOBS VERSION BUILD_DATE BUILD_SHA INPUT_VERSION CADCRAFT_VERSION CADCRAFT_BUILD_DATE CADCRAFT_BUILD_SHA ARTCRAFT_LAUNCHER_VERSION ARTCRAFT_LAUNCHER_BUILD_DATE ARTCRAFT_LAUNCHER_BUILD_SHA DECKCRAFT_VERSION DECKCRAFT_BUILD_DATE DECKCRAFT_BUILD_SHA DESIGNCRAFT_VERSION DESIGNCRAFT_BUILD_DATE DESIGNCRAFT_BUILD_SHA EFFECTCRAFT_VERSION EFFECTCRAFT_BUILD_DATE EFFECTCRAFT_BUILD_SHA FILMCRAFT_VERSION FILMCRAFT_BUILD_DATE FILMCRAFT_BUILD_SHA GRIDCRAFT_VERSION GRIDCRAFT_BUILD_DATE GRIDCRAFT_BUILD_SHA LIGHTCRAFT_VERSION LIGHTCRAFT_BUILD_DATE LIGHTCRAFT_BUILD_SHA PDFCRAFT_VERSION PDFCRAFT_BUILD_DATE PDFCRAFT_BUILD_SHA PHOTOCRAFT_VERSION PHOTOCRAFT_BUILD_DATE PHOTOCRAFT_BUILD_SHA SOUNDCRAFT_VERSION SOUNDCRAFT_BUILD_DATE SOUNDCRAFT_BUILD_SHA VECTORCRAFT_VERSION VECTORCRAFT_BUILD_DATE VECTORCRAFT_BUILD_SHA WORDCRAFT_VERSION WORDCRAFT_BUILD_DATE WORDCRAFT_BUILD_SHA FIGCRAFT_VERSION FIGCRAFT_BUILD_DATE FIGCRAFT_BUILD_SHA"
prepare: |
# Keep rustup/proxies outside the Cargo data cache.
unset CARGO_HOME CARGO_TARGET_DIR
@@ -740,18 +741,22 @@ jobs:
if [ -f packaging/freebsd/package.sh ]; then
bash packaging/freebsd/package.sh
else
cargo build --release --locked -p ${{ inputs.app-name }} $(cargo metadata --format-version 1 --no-deps | grep -q '"${{ inputs.app-name }}-cli"' && echo "-p ${{ inputs.app-name }}-cli" || true)
name="${{ inputs.app-name }}-$VERSION-freebsd-${{ matrix.arch }}"
set -- -p "$APP_NAME"
if cargo metadata --format-version 1 --no-deps | python3 -c 'import json, sys; sys.exit(not any(p["name"] == sys.argv[1] for p in json.load(sys.stdin)["packages"]))' "$APP_NAME-cli"; then
set -- "$@" -p "$APP_NAME-cli"
fi
cargo build --release --locked "$@"
name="$APP_NAME-$VERSION-freebsd-${{ matrix.arch }}"
mkdir -p "dist/release/$name"
cp target/release/${{ inputs.app-name }} "dist/release/$name/" 2>/dev/null || cp /var/tmp/craft-target/release/${{ inputs.app-name }} "dist/release/$name/"
if [ -f target/release/${{ inputs.app-name }}-cli ]; then
cp target/release/${{ inputs.app-name }}-cli "dist/release/$name/"
elif [ -f /var/tmp/craft-target/release/${{ inputs.app-name }}-cli ]; then
cp /var/tmp/craft-target/release/${{ inputs.app-name }}-cli "dist/release/$name/"
cp "target/release/$APP_NAME" "dist/release/$name/" 2>/dev/null || cp "/var/tmp/craft-target/release/$APP_NAME" "dist/release/$name/"
if [ -f "target/release/$APP_NAME-cli" ]; then
cp "target/release/$APP_NAME-cli" "dist/release/$name/"
elif [ -f "/var/tmp/craft-target/release/$APP_NAME-cli" ]; then
cp "/var/tmp/craft-target/release/$APP_NAME-cli" "dist/release/$name/"
fi
cp README.md LICENSE-MIT LICENSE-APACHE NOTICE ATTRIBUTION.md "dist/release/$name/" 2>/dev/null || true
if [ -f packaging/linux/ai.storyteller.${{ inputs.app-name }}.desktop ]; then
cp packaging/linux/ai.storyteller.${{ inputs.app-name }}.desktop "dist/release/$name/"
if [ -f "packaging/linux/ai.storyteller.$APP_NAME.desktop" ]; then
cp "packaging/linux/ai.storyteller.$APP_NAME.desktop" "dist/release/$name/"
fi
mkdir -p "dist/release/$name/icons"
cp -R assets/app-icon/hicolor "dist/release/$name/icons/" 2>/dev/null || true
@@ -1013,8 +1018,10 @@ jobs:
run: packaging/server/package.sh
- name: Check the archive
shell: bash
env:
APP_NAME: ${{ inputs.app-name }}
run: |
tar -tzf "dist/release/${{ inputs.app-name }}-server-$VERSION-linux-${{ matrix.arch }}.tar.gz" || true
tar -tzf "dist/release/$APP_NAME-server-$VERSION-linux-${{ matrix.arch }}.tar.gz" || true
- uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2
with:
name: release-server-linux-${{ matrix.arch }}
tests/test_freebsd_release.py
+18 -4
View File
@@ -4,6 +4,7 @@ from pathlib import Path
import shutil
import subprocess
import sys
import tarfile
import tempfile
import unittest
@@ -18,7 +19,7 @@ COPY = next(step['run'] for step in STEPS if step.get('name') == 'Copy release a
@unittest.skipUnless(shutil.which('rsync'), 'rsync required')
class FreeBSDReleaseTests(unittest.TestCase):
def run_release(self, fail_build=False):
def run_release(self, fail_build=False, with_cli=False):
with tempfile.TemporaryDirectory() as directory:
root = Path(directory)
guest, host, tools = (root / name for name in ('guest', 'host', 'tools'))
@@ -30,10 +31,14 @@ class FreeBSDReleaseTests(unittest.TestCase):
cargo = tools / 'cargo'
cargo.write_text('''#!/bin/sh
set -eu
if [ "$1" = metadata ]; then echo '{}'; exit 0; fi
if [ "$1" = metadata ]; then
if [ "$WITH_CLI" = true ]; then echo '{"packages":[{"name":"fixture-cli"}]}'; else echo '{"packages":[]}'; fi
exit 0
fi
[ "$FAIL_BUILD" = false ] || exit 31
mkdir -p "$CARGO_TARGET_DIR/release"
echo binary > "$CARGO_TARGET_DIR/release/fixture"
if [ "$WITH_CLI" = true ]; then echo cli > "$CARGO_TARGET_DIR/release/fixture-cli"; fi
''')
cargo.chmod(0o755)
git = tools / 'git'
@@ -53,7 +58,7 @@ sys.exit(subprocess.run([os.environ['REAL_RSYNC'], *args]).returncode)
env = dict(os.environ, PATH=str(tools) + os.pathsep + os.environ['PATH'],
CARGO_TARGET_DIR=str(target), GITHUB_WORKSPACE=str(guest),
REAL_RSYNC=shutil.which('rsync'), VERSION='1.0.0', INPUT_VERSION='',
FAIL_BUILD=str(fail_build).lower())
FAIL_BUILD=str(fail_build).lower(), WITH_CLI=str(with_cli).lower(), APP_NAME='fixture')
script = BUILD.replace('${{ inputs.app-name }}', 'fixture').replace('${{ matrix.arch }}', 'x86_64')
script = script.replace('${{ steps.cargo-cache.outcome }}', 'failure')
script = script.replace('/var/tmp/', str(guest) + '/')
@@ -63,11 +68,20 @@ sys.exit(subprocess.run([os.environ['REAL_RSYNC'], *args]).returncode)
return result.returncode, False, (target / 'release/partial').exists()
copied = subprocess.run(['bash', '-eu', '-c', COPY], cwd=host, env=env, capture_output=True, text=True)
archive = host / 'dist/release/fixture-1.0.0-freebsd-x86_64.tar.gz'
return copied.returncode, archive.is_file(), (target / 'release/partial').exists()
with tarfile.open(archive) as packaged:
names = packaged.getnames()
prefix = 'fixture-1.0.0-freebsd-x86_64/'
complete = prefix + 'fixture' in names
if with_cli:
complete = complete and prefix + 'fixture-cli' in names
return copied.returncode, complete, (target / 'release/partial').exists()
def test_failed_restore_rebuilds_and_copies_real_archive(self):
self.assertEqual(self.run_release(), (0, True, False))
def test_optional_cli_is_packaged_from_cached_target(self):
self.assertEqual(self.run_release(with_cli=True), (0, True, False))
def test_failed_build_never_delivers_archive(self):
self.assertEqual(self.run_release(fail_build=True), (31, False, False))
tests/test_release_inputs.py
+110
View File
@@ -0,0 +1,110 @@
"""Exercise the release workflow's app-name boundary with fixture commands."""
import json
import os
from pathlib import Path
import subprocess
import sys
import tempfile
import unittest
import yaml
ROOT = Path(__file__).resolve().parents[1]
WORKFLOW = ROOT / '.github/workflows/release.yml'
DATA = yaml.load(WORKFLOW.read_text(), Loader=yaml.BaseLoader)
EXPRESSION = '${{ inputs.app-name }}'
class ReleaseInputTests(unittest.TestCase):
def run_step(self, job_name, step_name, app_name, cli=False):
job = DATA['jobs'][job_name]
step = next(step for step in job['steps'] if step.get('name') == step_name)
with tempfile.TemporaryDirectory() as directory:
root = Path(directory)
tools = root / 'tools'
tools.mkdir()
for name, source in {
'git': 'import sys\nsys.exit(0)\n',
'cargo': '''import json, os, sys
from pathlib import Path
if sys.argv[1] == 'metadata':
names = [os.environ['TEST_APP_NAME']]
if os.environ['TEST_CLI'] == 'true':
names.append(os.environ['TEST_APP_NAME'] + '-cli')
print(json.dumps({'packages': [{'name': name} for name in names]}))
else:
Path('args.json').write_text(json.dumps(sys.argv[1:]))
sys.exit(31) # Stop before packaging; real archives are covered separately.
''',
'tar': '''import json, sys
from pathlib import Path
Path('args.json').write_text(json.dumps(sys.argv[1:]))
''',
}.items():
tool = tools / name
tool.write_text(f'#!{sys.executable}\n' + source)
tool.chmod(0o755)
env = dict(os.environ, PATH=str(tools) + os.pathsep + os.environ['PATH'],
HOME=str(root), TEST_APP_NAME=app_name, TEST_CLI=str(cli).lower())
configured = {**job.get('env', {}), **step.get('env', {})}
if job_name == 'freebsd':
vm = next(item for item in job['steps']
if item.get('uses', '').startswith('vmactions/freebsd-vm@'))
forwarded = vm['with']['envs'].split()
configured = {key: value for key, value in configured.items() if key in forwarded}
env.update({key: value.replace(EXPRESSION, app_name)
for key, value in configured.items()})
env.update(VERSION='1.2.3', INPUT_VERSION='')
# Model GitHub expression replacement in source as well as env so a
# regression to interpolation executes the payload in this fixture.
script = step['run'].replace(EXPRESSION, app_name)
script = script.replace('${{ matrix.arch }}', 'x86_64')
script = script.replace('${{ steps.cargo-cache.outcome }}', 'success')
script = script.replace('/var/tmp/', str(root) + '/')
shell = 'sh' if job_name == 'freebsd' else 'bash'
result = subprocess.run([shell, '-eu', '-c', script], cwd=root, env=env,
capture_output=True, text=True, timeout=10)
log = root / 'args.json'
return result, json.loads(log.read_text()) if log.exists() else None, (root / 'injected').exists()
def test_freebsd_package_arguments(self):
for app_name in ('pdfcraft', 'craft-launcher', 'Craft_2'):
for cli in (False, True):
with self.subTest(app_name=app_name, cli=cli):
result, args, injected = self.run_step('freebsd', 'Build and package', app_name, cli)
self.assertEqual(result.returncode, 31, result.stderr)
expected = ['build', '--release', '--locked', '-p', app_name]
if cli:
expected.extend(['-p', app_name + '-cli'])
self.assertEqual(args, expected)
self.assertFalse(injected)
def test_app_name_is_literal_at_both_shell_sinks(self):
for app_name in (
'fixture$(touch injected)', 'fixture`touch injected`',
'fixture; touch injected; #', 'fixture"; touch injected; #',
"fixture'; touch injected; #", 'fixture\ntouch injected\n#',
'fixture --features injected', 'fixture*',
):
for job, step in (('freebsd', 'Build and package'), ('server', 'Check the archive')):
with self.subTest(app_name=app_name, job=job):
result, args, injected = self.run_step(job, step, app_name)
self.assertFalse(injected, result.stderr)
if job == 'freebsd':
self.assertEqual(result.returncode, 31, result.stderr)
self.assertEqual(args, ['build', '--release', '--locked', '-p', app_name])
else:
self.assertEqual(result.returncode, 0, result.stderr)
self.assertEqual(args, ['-tzf', f'dist/release/{app_name}-server-1.2.3-linux-x86_64.tar.gz'])
def test_server_archive_name_is_preserved(self):
result, args, injected = self.run_step('server', 'Check the archive', 'pdfcraft')
self.assertEqual(result.returncode, 0, result.stderr)
self.assertEqual(args, ['-tzf', 'dist/release/pdfcraft-server-1.2.3-linux-x86_64.tar.gz'])
self.assertFalse(injected)
if __name__ == '__main__':
unittest.main()