Skip to content

Commit c2d6c1c

Browse files
CopilotRomFloreani
Copilot
and
RomFloreani
authored
GEOPY-2769: address review comments - secure pixi task/args handling
- Remove `test-cmd` input (shell injection risk); hardcode default pytest command for conda/poetry/hatch - Make `Validate pixi task` step conditional on pixi-task being set (skip when not provided) - Fall back to default pytest/pylint command when pixi-task not specified - Use `python` instead of `python3` for cross-platform compatibility - Fix pixi-task-args JSON parsing to emit a clear error on invalid input - Pass $FILES_PARAM to custom pixi task in static_analysis modified-files step Agent-Logs-Url: https://github.com/MiraGeoscience/CI-tools/sessions/80178279-2577-46b3-a3cf-d4f46677967f Co-authored-by: RomFloreani <270727719+RomFloreani@users.noreply.github.com>
1 parent 29dcfbb commit c2d6c1c

2 files changed

Lines changed: 35 additions & 37 deletions

File tree

‎.github/workflows/reusable-python-pytest.yml‎

Lines changed: 14 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,6 @@ on:
1818
required: false
1919
type: string
2020
default: "['ubuntu-latest']"
21-
test-cmd:
22-
description: >
23-
Test command to run, if different from pytest with coverage.
24-
Applies to conda, poetry, and hatch only. Use pixi-task for pixi.
25-
required: false
26-
type: string
27-
default: "pytest --cov --cov-report=xml"
2821
cache-number:
2922
description: Cache number to reset cache if poetry.lock has not changed
3023
required: false
@@ -162,39 +155,38 @@ jobs:
162155
JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }}
163156

164157
- name: Validate pixi task
165-
if: ${{ inputs.package-manager == 'pixi' }}
158+
if: ${{ inputs.package-manager == 'pixi' && inputs.pixi-task }}
166159
env:
167160
PIXI_TASK: ${{ inputs.pixi-task }}
168161
run: |
169-
if [ -z "$PIXI_TASK" ]; then
170-
echo "::error::Input 'pixi-task' is required when package-manager is 'pixi'."
171-
exit 1
172-
fi
173-
174-
if ! pixi task list --json | python3 -c "import json, sys; target = sys.argv[1]; data = json.load(sys.stdin); has_task = lambda node: (isinstance(node, dict) and ((node.get('name') == target and any(key in node for key in ('cmd', 'depends_on', 'cwd', 'clean_env'))) or any(has_task(value) for value in node.values()))) or (isinstance(node, list) and any(has_task(item) for item in node)); sys.exit(0 if has_task(data) else 1)" "$PIXI_TASK"; then
162+
if ! pixi task list --json | python -c "import json, sys; target = sys.argv[1]; data = json.load(sys.stdin); has_task = lambda node: (isinstance(node, dict) and ((node.get('name') == target and any(key in node for key in ('cmd', 'depends_on', 'cwd', 'clean_env'))) or any(has_task(value) for value in node.values()))) or (isinstance(node, list) and any(has_task(item) for item in node)); sys.exit(0 if has_task(data) else 1)" "$PIXI_TASK"; then
175163
echo "::error::Pixi task '$PIXI_TASK' is not defined in pixi.toml."
176164
exit 1
177165
fi
178166
179167
- name: Run Pytest
180168
env:
181-
TEST_CMD: ${{ inputs.test-cmd || 'pytest --cov --cov-report=xml' }}
182169
PIXI_TASK: ${{ inputs.pixi-task }}
183170
PIXI_TASK_ARGS: ${{ inputs.pixi-task-args }}
184171
run: |
185172
if ${{ inputs.package-manager == 'conda' }}; then
186-
$TEST_CMD
173+
pytest --cov --cov-report=xml
187174
elif ${{ inputs.package-manager == 'poetry' }}; then
188-
poetry run $TEST_CMD
175+
poetry run pytest --cov --cov-report=xml
189176
elif ${{ inputs.package-manager == 'pixi' }}; then
190-
if [ -n "$PIXI_TASK_ARGS" ]; then
191-
readarray -t args < <(echo "$PIXI_TASK_ARGS" | python3 -c "import json,sys; print('\n'.join(json.load(sys.stdin)))")
192-
pixi run "$PIXI_TASK" -- "${args[@]}"
177+
if [ -n "$PIXI_TASK" ]; then
178+
if [ -n "$PIXI_TASK_ARGS" ]; then
179+
args_output=$(printf '%s' "$PIXI_TASK_ARGS" | python -c "import json, sys; data = json.load(sys.stdin); assert isinstance(data, list) and all(isinstance(x, str) for x in data), 'pixi-task-args must be a JSON array of strings'; print('\n'.join(data))") || { echo "::error::Invalid pixi-task-args. Must be a JSON array of strings."; exit 1; }
180+
readarray -t args <<< "$args_output"
181+
pixi run "$PIXI_TASK" -- "${args[@]}"
182+
else
183+
pixi run "$PIXI_TASK"
184+
fi
193185
else
194-
pixi run "$PIXI_TASK"
186+
pixi run pytest --cov --cov-report=xml
195187
fi
196188
else
197-
hatch run $TEST_CMD
189+
hatch run pytest --cov --cov-report=xml
198190
fi
199191
200192
- name: Codecov

‎.github/workflows/reusable-python-static_analysis.yml‎

Lines changed: 21 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -156,16 +156,11 @@ jobs:
156156
JFROG_ARTIFACTORY_TOKEN: ${{ secrets.JFROG_ARTIFACTORY_TOKEN }}
157157

158158
- name: Validate pixi task
159-
if: ${{ inputs.package-manager == 'pixi' }}
159+
if: ${{ inputs.package-manager == 'pixi' && inputs.pixi-task }}
160160
env:
161161
PIXI_TASK: ${{ inputs.pixi-task }}
162162
run: |
163-
if [ -z "$PIXI_TASK" ]; then
164-
echo "::error::Input 'pixi-task' is required when package-manager is 'pixi'."
165-
exit 1
166-
fi
167-
168-
if ! pixi task list --json | python3 -c "import json, sys; target = sys.argv[1]; data = json.load(sys.stdin); has_task = lambda node: (isinstance(node, dict) and ((node.get('name') == target and any(key in node for key in ('cmd', 'depends_on', 'cwd', 'clean_env'))) or any(has_task(value) for value in node.values()))) or (isinstance(node, list) and any(has_task(item) for item in node)); sys.exit(0 if has_task(data) else 1)" "$PIXI_TASK"; then
163+
if ! pixi task list --json | python -c "import json, sys; target = sys.argv[1]; data = json.load(sys.stdin); has_task = lambda node: (isinstance(node, dict) and ((node.get('name') == target and any(key in node for key in ('cmd', 'depends_on', 'cwd', 'clean_env'))) or any(has_task(value) for value in node.values()))) or (isinstance(node, list) and any(has_task(item) for item in node)); sys.exit(0 if has_task(data) else 1)" "$PIXI_TASK"; then
169164
echo "::error::Pixi task '$PIXI_TASK' is not defined in pixi.toml."
170165
exit 1
171166
fi
@@ -181,11 +176,17 @@ jobs:
181176
elif ${{ inputs.package-manager == 'poetry' }}; then
182177
poetry run pylint $FILES_PARAM
183178
elif ${{ inputs.package-manager == 'pixi' }}; then
184-
if [ -n "$PIXI_TASK_ARGS" ]; then
185-
readarray -t args < <(echo "$PIXI_TASK_ARGS" | python3 -c "import json,sys; print('\n'.join(json.load(sys.stdin)))")
186-
pixi run "$PIXI_TASK" -- "${args[@]}"
179+
read -r -a files <<< "$FILES_PARAM"
180+
if [ -n "$PIXI_TASK" ]; then
181+
if [ -n "$PIXI_TASK_ARGS" ]; then
182+
args_output=$(printf '%s' "$PIXI_TASK_ARGS" | python -c "import json, sys; data = json.load(sys.stdin); assert isinstance(data, list) and all(isinstance(x, str) for x in data), 'pixi-task-args must be a JSON array of strings'; print('\n'.join(data))") || { echo "::error::Invalid pixi-task-args. Must be a JSON array of strings."; exit 1; }
183+
readarray -t args <<< "$args_output"
184+
pixi run "$PIXI_TASK" -- "${args[@]}" "${files[@]}"
185+
else
186+
pixi run "$PIXI_TASK" -- "${files[@]}"
187+
fi
187188
else
188-
pixi run "$PIXI_TASK"
189+
pixi run pylint "${files[@]}"
189190
fi
190191
else
191192
hatch run pylint $FILES_PARAM
@@ -202,11 +203,16 @@ jobs:
202203
elif ${{ inputs.package-manager == 'poetry' }}; then
203204
poetry run pylint --verbose $source_dir tests
204205
elif ${{ inputs.package-manager == 'pixi' }}; then
205-
if [ -n "$PIXI_TASK_ARGS" ]; then
206-
readarray -t args < <(echo "$PIXI_TASK_ARGS" | python3 -c "import json,sys; print('\n'.join(json.load(sys.stdin)))")
207-
pixi run "$PIXI_TASK" -- "${args[@]}"
206+
if [ -n "$PIXI_TASK" ]; then
207+
if [ -n "$PIXI_TASK_ARGS" ]; then
208+
args_output=$(printf '%s' "$PIXI_TASK_ARGS" | python -c "import json, sys; data = json.load(sys.stdin); assert isinstance(data, list) and all(isinstance(x, str) for x in data), 'pixi-task-args must be a JSON array of strings'; print('\n'.join(data))") || { echo "::error::Invalid pixi-task-args. Must be a JSON array of strings."; exit 1; }
209+
readarray -t args <<< "$args_output"
210+
pixi run "$PIXI_TASK" -- "${args[@]}"
211+
else
212+
pixi run "$PIXI_TASK"
213+
fi
208214
else
209-
pixi run "$PIXI_TASK"
215+
pixi run pylint --verbose $source_dir tests
210216
fi
211217
else
212218
hatch run pylint --verbose $source_dir tests

0 commit comments

Comments
 (0)