Skip to content

Create a Dev Container Feature - #10

Open
skeymeulen wants to merge 4 commits into
mainfrom
devcontainer-secrets-support
Open

Create a Dev Container Feature#10
skeymeulen wants to merge 4 commits into
mainfrom
devcontainer-secrets-support

Conversation

@skeymeulen

@skeymeulen skeymeulen commented Jan 21, 2026

Copy link
Copy Markdown
Member

Summary by Aikido

⚠️ Security Issues: 2 🔍 Quality Issues: 2 Resolved Issues: 0

🚀 New Features

  • Added Dev Container Feature with install script and smoke tests

⚡ Enhancements

  • Added workflow to publish devcontainer features with docs disabled

More info

@Datacom-Ayden

Copy link
Copy Markdown

Hi There, how are you going.
Floated the feature request, and I've built a working Dev Container feature to help.

I've tested it within my projects using the Aikido local scanner.
aikido-precommit feature
aikido-precommit feature test

It's currently working off the install-aikido-hook.sh as a reference.
But it will have versioning issues unless Dependabot is updated to grab the version and update the feature.

# Get options from environment (feature options are uppercase)
VERSION="${VERSION:-"v1.0.116"}"
SETUP_GLOBAL_HOOKS="${SETUPGLOBALHOOKS:-"true"}"

...

# Construct download URL
BASE_URL="https://aikido-local-scanner.s3.eu-west-1.amazonaws.com/${VERSION}"
BINARY_NAME="aikido-local-scanner"
DOWNLOAD_FILE="${BINARY_NAME}.zip"
DOWNLOAD_URL="${BASE_URL}/${PLATFORM}_${ARCH_NAME}/${DOWNLOAD_FILE}"

I think similar to #8, if version tags are set up version management would be easier.

@Datacom-Ayden

Copy link
Copy Markdown

I've setup some version mgmt to detect the version, while still allowing for specific version to be specified

permissions:
packages: write
steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitHub Action actions/checkout persist Git credentials in workflow - low severity
actions/checkout v2 and above persist the default GITHUB_TOKEN in the repository's local git config when persist-credentials is not set to false, during the workflow run. Subsequent workflow steps or third-party actions can read this token from git configuration, increasing the risk of credential theft or misuse within the pipeline. In order to limit the attack surface when external actions are compromised, ensure persist-credentials is set to false.

Show fix

Remediation: Set persist-credentials: false on actions/checkout steps that do not need to push commits back to the repository. Only keep persist-credentials: true when the workflow explicitly performs authenticated git push operations.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

smoke-test:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GitHub Action actions/checkout persist Git credentials in workflow - low severity
actions/checkout v2 and above persist the default GITHUB_TOKEN in the repository's local git config when persist-credentials is not set to false, during the workflow run. Subsequent workflow steps or third-party actions can read this token from git configuration, increasing the risk of credential theft or misuse within the pipeline. In order to limit the attack surface when external actions are compromised, ensure persist-credentials is set to false.

Show fix

Remediation: Set persist-credentials: false on actions/checkout steps that do not need to push commits back to the repository. Only keep persist-credentials: true when the workflow explicitly performs authenticated git push operations.

Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

tmp_dir="$(mktemp -d)"
trap 'rm -rf "$tmp_dir"' RETURN

log "Downloading ${BINARY_NAME} ${VERSION} (${PLATFORM})..."

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Logging VERSION and PLATFORM may expose user-supplied or environment values; avoid printing raw VERSION/PLATFORM or sanitize/mask before logging.

Details

✨ AI Reasoning
​The script logs the download action including VERSION and PLATFORM. VERSION can be supplied by a user or feature option, and PLATFORM is derived from uname but may expose environment details. Logging these unsanitized values can leak user-controlled input or environment specifics to log storage and viewers. The logged data appears directly in a single log() call and in the final echo; no sanitization or masking is applied. This was introduced by the new install script and increases exposure of configuration input in logs.

🔧 How do I fix it?
Keep sensitive data such as emails, passwords, and tokens out of logs. When logging values tied to a user, prefer a safe identifier like a user ID over the raw input, and strip line breaks from any user-provided text you do log.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

# --- Version resolution ------------------------------------------------------
resolve_version() {
# Fall back to the pinned default when no explicit version is requested.
[ -n "$VERSION" ] || VERSION="$FALLBACK_VERSION"

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

resolve_version uses '[ -n "$VERSION" ] || VERSION="$FALLBACK_VERSION"'. Prefer the simpler, idiomatic parameter expansion: VERSION="${VERSION:-$FALLBACK_VERSION}".

Details

✨ AI Reasoning
​The code aims to assign a default FALLBACK_VERSION to VERSION when VERSION is empty. It currently performs a conditional test and assignment using [ -n "$VERSION" ] || VERSION="$FALLBACK_VERSION". This is more verbose than the common shell parameter expansion idiom (VERSION="${VERSION:-$FALLBACK_VERSION}"), which is behavior-equivalent, clearer, and idiomatic in POSIX/bash scripts. The change introduces verbosity without semantic benefit.

🔧 How do I fix it?
Rewrite the snippet in the simpler, behavior-equivalent form: return a boolean expression directly instead of if cond return true else return false, avoid using lists when they are guaranteed to contain one element, etc.

Reply @AikidoSec feedback: [FEEDBACK] to get better review comments in the future.
Reply @AikidoSec ignore: [REASON] to ignore this issue.
More info

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants