Skip to content

Add govulncheck CI workflow#97

Open
Arnaud M. (ameukam) wants to merge 1 commit into
agent-substrate:mainfrom
ameukam:add-govulncheck-workflow
Open

Add govulncheck CI workflow#97
Arnaud M. (ameukam) wants to merge 1 commit into
agent-substrate:mainfrom
ameukam:add-govulncheck-workflow

Conversation

@ameukam
Copy link
Copy Markdown
Contributor

No description provided.

@ameukam
Copy link
Copy Markdown
Contributor Author

Comment thread .github/workflows/govulncheck.yaml Outdated
Comment thread .github/workflows/govulncheck.yaml Outdated

name: govulncheck
on:
pull_request:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it might be more useful to run periodically on the merged code, since vulns can appear even with no pull request, and probably aren't related to the pull request?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added a cron.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think we should drop the PR flow as well, so it doesn't block PRs while we remediate.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

runs-on: ubuntu-latest
steps:
- id: govulncheck
uses: golang/govulncheck-action@v1
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

does this do symbol level by default, or only package level?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I believe it does symbol level by default.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, I'd like to make sure it is symbol level to cut down noise.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

AIUI in Kubernetes it is only running at the package level.

@ameukam Arnaud M. (ameukam) force-pushed the add-govulncheck-workflow branch from 646d703 to 819c706 Compare May 28, 2026 03:04
@BenTheElder Benjamin Elder (BenTheElder) added the test Enhancing / fixing test coverage. label May 28, 2026
Signed-off-by: Arnaud Meukam <ameukam@gmail.com>
@ameukam Arnaud M. (ameukam) force-pushed the add-govulncheck-workflow branch from 819c706 to 516cecb Compare May 28, 2026 03:51
@a4-a4s1
Copy link
Copy Markdown

a4-a4s1 Bot commented May 28, 2026

Skim — clean addition; the minimal-scope permissions: contents: read is right.

One observation worth raising before this lands:

  • Trigger is schedule-only. Weekly Mondays at 04:37 UTC catches CVEs published in the prior week, but does NOT catch vulnerabilities introduced by dependency-bump PRs at PR-time — dependabot (or any contributor) opens a PR pulling vulnerable-transitive-X, it merges before next Monday's run, and the project ships the vulnerable version for up to ~6 days before govulncheck flags it. Most security-conscious repos pair schedule: with pull_request: so dep-bump PRs get gated by the same check:
    on:
      schedule:
        - cron: "37 4 * * 1"
      pull_request:
        paths:
          - 'go.mod'
          - 'go.sum'
          - '**.go'
    govulncheck exits non-zero on found vulns, so under branch protection this becomes a merge gate.

Q: is the schedule-only shape intentional (e.g. to defer the cost of a PR-time check until governance lands), or would you accept a follow-up adding the PR-trigger?

name: govulncheck
on:
schedule:
- cron: "37 4 * * 1"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

let's also run on push to main

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

Labels

test Enhancing / fixing test coverage.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants