Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions danger/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ jobs:
* type: string
* required: false
* default: `${{ github.token }}`
* extra-dangerfile: Path to an additional dangerfile to run custom checks.
* extra-install-packages: Additional packages that are required by the extra-dangerfile, you can find a list of packages here: https://packages.debian.org/search?suite=bookworm&keywords=curl.
Comment thread
seer-by-sentry[bot] marked this conversation as resolved.
Outdated

## Outputs

Expand Down
49 changes: 44 additions & 5 deletions danger/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ inputs:
description: 'Token for the repo. Can be passed in using {{ secrets.GITHUB_TOKEN }}'
required: false
default: ${{ github.token }}
extra-dangerfile:
description: 'Path to additional dangerfile to run after the main checks'
type: string
required: false
extra-install-packages:
description: 'Additional apt packages to install in the DangerJS container (space-separated package names)'
type: string
required: false

outputs:
outcome:
Expand All @@ -28,12 +36,25 @@ runs:
shell: pwsh
run: Get-Content '${{ github.action_path }}/danger.properties' | Tee-Object $env:GITHUB_OUTPUT -Append

# Using a pre-built docker image in GitHub container registry instead of NPM to reduce possible attack vectors.
- name: Run DangerJS
id: danger
# Validate extra-install-packages to prevent code injection
- name: Validate package names
if: ${{ inputs.extra-install-packages }}
shell: bash
run: |
docker run \
packages="${{ inputs.extra-install-packages }}"
# Only allow alphanumeric characters, hyphens, periods, plus signs, underscores, and spaces
if ! echo "$packages" | grep -E '^[a-zA-Z0-9._+-]+( [a-zA-Z0-9._+-]+)*$' > /dev/null; then
echo "::error::Invalid package names in extra-install-packages. Only alphanumeric characters, hyphens, periods, plus signs, underscores, and spaces are allowed."
exit 1
fi

# Using a pre-built docker image in GitHub container registry instead of NPM to reduce possible attack vectors.
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
- name: Setup container
shell: bash
run: |
# Start a detached container with all necessary volumes and environment variables
docker run -td --name danger \
--entrypoint /bin/bash \
--volume ${{ github.workspace }}:/github/workspace \
--volume ${{ github.action_path }}:${{ github.action_path }} \
--volume ${{ github.event_path }}:${{ github.event_path }} \
Expand All @@ -42,5 +63,23 @@ runs:
-e "INPUT_ARGS" -e "GITHUB_JOB" -e "GITHUB_REF" -e "GITHUB_SHA" -e "GITHUB_REPOSITORY" -e "GITHUB_REPOSITORY_OWNER" -e "GITHUB_RUN_ID" -e "GITHUB_RUN_NUMBER" -e "GITHUB_RETENTION_DAYS" -e "GITHUB_RUN_ATTEMPT" -e "GITHUB_ACTOR" -e "GITHUB_TRIGGERING_ACTOR" -e "GITHUB_WORKFLOW" -e "GITHUB_HEAD_REF" -e "GITHUB_BASE_REF" -e "GITHUB_EVENT_NAME" -e "GITHUB_SERVER_URL" -e "GITHUB_API_URL" -e "GITHUB_GRAPHQL_URL" -e "GITHUB_REF_NAME" -e "GITHUB_REF_PROTECTED" -e "GITHUB_REF_TYPE" -e "GITHUB_WORKSPACE" -e "GITHUB_ACTION" -e "GITHUB_EVENT_PATH" -e "GITHUB_ACTION_REPOSITORY" -e "GITHUB_ACTION_REF" -e "GITHUB_PATH" -e "GITHUB_ENV" -e "GITHUB_STEP_SUMMARY" -e "RUNNER_OS" -e "RUNNER_ARCH" -e "RUNNER_NAME" -e "RUNNER_TOOL_CACHE" -e "RUNNER_TEMP" -e "RUNNER_WORKSPACE" -e "ACTIONS_RUNTIME_URL" -e "ACTIONS_RUNTIME_TOKEN" -e "ACTIONS_CACHE_URL" -e GITHUB_ACTIONS=true -e CI=true \
-e GITHUB_TOKEN="${{ inputs.api-token }}" \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Security: Direct Input Interpolation

This line directly interpolates inputs.extra-dangerfile into the environment variable assignment. According to repository security patterns, user-controlled inputs should be assigned to job-level env variables first to prevent injection attacks.

Recommendation: Add a job-level env block and reference it as ${{ env.EXTRA_DANGERFILE }}.

Did we get this right? 👍 / 👎 to inform future reviews.

-e DANGER_DISABLE_TRANSPILATION="true" \
-e EXTRA_DANGERFILE_INPUT="${{ inputs.extra-dangerfile }}" \
ghcr.io/danger/danger-js:${{ steps.config.outputs.version }} \
ghcr.io/danger/danger-js:${{ steps.config.outputs.version }} \
--failOnErrors --dangerfile ${{ github.action_path }}/dangerfile.js
-c "sleep infinity"

Comment thread
lucas-zimerman marked this conversation as resolved.
- name: Setup additional packages
if: ${{ inputs.extra-install-packages }}
shell: bash
run: |
docker exec --user root danger apt-get update
echo "Installing packages: ${{ inputs.extra-install-packages }}"
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
docker exec --user root danger sh -c "apt-get install -y ${{ inputs.extra-install-packages }}"
echo "All additional packages installed successfully."
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
Comment thread
lucas-zimerman marked this conversation as resolved.

- name: Run DangerJS
id: danger
shell: bash
run: |
trap "docker rm -f danger || true" EXIT
Comment thread
vaind marked this conversation as resolved.
Outdated
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
docker exec --user $(id -u) danger danger ci --fail-on-errors --dangerfile ${{ github.action_path }}/dangerfile.js
27 changes: 27 additions & 0 deletions danger/dangerfile.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
const { getFlavorConfig, extractPRFlavor } = require('./dangerfile-utils.js');
const fs = require('fs');

const headRepoName = danger.github.pr.head.repo.git_url;
const baseRepoName = danger.github.pr.base.repo.git_url;
Expand Down Expand Up @@ -186,10 +187,36 @@ async function checkActionsArePinned() {
}
}

async function CheckFromExternalChecks() {
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
// Get the external dangerfile path from environment variable (passed via workflow input)
// Priority: EXTRA_DANGERFILE (absolute path) -> EXTRA_DANGERFILE_INPUT (relative path)
const customPath = process.env.EXTRA_DANGERFILE || process.env.EXTRA_DANGERFILE_INPUT;
console.log(`::debug:: Checking from external checks: ${customPath}`);
if (customPath) {
try {
Comment thread
seer-by-sentry[bot] marked this conversation as resolved.
const extraModule = require(`/github/workspace/${customPath}`);
await extraModule({
fail: fail,
warn: warn,
message: message,
markdown: markdown,
danger: danger,
});
} catch (err) {
if (err.message && err.message.includes('Cannot use import statement outside a module')) {
warn(`External dangerfile uses ES6 imports. Please convert to CommonJS syntax (require/module.exports) or use .mjs extension with proper module configuration.\nFile: ${customPath}`);
Comment thread
seer-by-sentry[bot] marked this conversation as resolved.
Outdated
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
} else {
warn(`Could not load custom Dangerfile: ${customPath}\n${err}`);
}
}
}
}

async function checkAll() {
await checkDocs();
await checkChangelog();
await checkActionsArePinned();
await CheckFromExternalChecks();
Comment thread
lucas-zimerman marked this conversation as resolved.
Outdated
}

schedule(checkAll);
Loading