Skip to content

feat: add reusable secrets-scan workflow + template#91

Open
fibble wants to merge 1 commit intomainfrom
vol-6349-secrets-scan
Open

feat: add reusable secrets-scan workflow + template#91
fibble wants to merge 1 commit intomainfrom
vol-6349-secrets-scan

Conversation

@fibble
Copy link
Copy Markdown
Contributor

@fibble fibble commented Apr 28, 2026

Description

Shared github action to run git-secrets

Before submitting (or marking as "ready for review")

  • Does the pull request title follow the conventional commit specification?
  • Have you performed a self-review of the code
  • Have you have added tests that prove the fix or feature is effective and working
  • Did you make sure to update any documentation relating to this change?

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds a reusable GitHub Actions workflow and a repository workflow-template for running awslabs/git-secrets to detect committed secrets across DVSA repositories.

Changes:

  • Introduces a new reusable workflow (workflow_call) to install and run git-secrets against the working tree and optionally full history.
  • Adds a workflow template to run the reusable secrets-scan on PRs, pushes, and a scheduled cron.
  • Adds the template metadata (properties.json) so the workflow is available via GitHub’s workflow templates UI.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
workflow-templates/secrets-scan.yaml New workflow template wiring repo events to the shared reusable secrets scan workflow.
workflow-templates/secrets-scan.properties.json Template metadata (name/description/icon/categories) for GitHub workflow templates.
.github/workflows/secrets-scan.yaml New reusable workflow that installs and runs awslabs/git-secrets with AWS patterns.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +33 to +36
- name: Checkout (full history)
uses: actions/checkout@v6
with:
fetch-depth: 0
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The workflow always checks out full git history (fetch-depth: 0) even when inputs.scan-history is false, which undermines the “fast on PRs” intent and can be slow for large repositories. Consider using a conditional/parameterized fetch-depth (0 only when scan-history is true; otherwise default/shallow).

Suggested change
- name: Checkout (full history)
uses: actions/checkout@v6
with:
fetch-depth: 0
- name: Checkout
uses: actions/checkout@v6
with:
fetch-depth: ${{ inputs.scan-history && 0 || 1 }}

Copilot uses AI. Check for mistakes.
run: |
set -euo pipefail
tmp="$(mktemp -d)"
git clone --quiet --depth 1 https://github.com/awslabs/git-secrets "$tmp"
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This installs git-secrets by cloning and executing build scripts from GitHub at runtime and then running sudo make install. To reduce supply-chain risk and improve reproducibility, pin the clone to a specific tag/commit (or use a packaged/released artifact with integrity verification) rather than the repo’s moving default branch.

Suggested change
git clone --quiet --depth 1 https://github.com/awslabs/git-secrets "$tmp"
git clone --quiet --depth 1 --branch 1.3.0 --single-branch https://github.com/awslabs/git-secrets "$tmp"

Copilot uses AI. Check for mistakes.
on:
pull_request:
push:
branches: [main, master]
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

This template hard-codes branches: [main, master] for push. Other workflow templates in this repo use the $default-branch placeholder so the template works in repos whose default branch is neither main nor master (e.g. workflow-templates/php-security.yml:5). Consider switching to branches: [ $default-branch ] for consistency and correctness.

Suggested change
branches: [main, master]
branches: [ $default-branch ]

Copilot uses AI. Check for mistakes.

jobs:
secrets:
uses: dvsa/.github/.github/workflows/secrets-scan.yaml@main
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The template references the reusable workflow with @main. In this repo’s other templates, reusable workflows are pinned to a released version tag (e.g. workflow-templates/php-security.yml:13 uses @v5.0.6) to avoid unexpected breaking changes for consumers. Consider pinning to a version tag and updating it on release.

Suggested change
uses: dvsa/.github/.github/workflows/secrets-scan.yaml@main
uses: dvsa/.github/.github/workflows/secrets-scan.yaml@v5.0.6

Copilot uses AI. Check for mistakes.
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.

yes would want to do that @fibble

@sdh100shaun
Copy link
Copy Markdown
Contributor

sdh100shaun commented Apr 28, 2026

@fibble not sure what your ticket says but we should have this on pre-commit hooks too so this is very much a belt and braces , in open source circumsatnces obviously if has hit the remote
repo it is too late

@fibble
Copy link
Copy Markdown
Contributor Author

fibble commented Apr 28, 2026

@fibble not sure what your ticket says but we should have this on pre-commit hooks too so this is very much a belt and braces , in open source circumsatnces obviously if has hit the remote repo it is too late

Yes, it's also added to 9 repositories as precommit hooks..this is a backstop incase of --no-verify bypass etc

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.

3 participants