Skip to content

fix: correctly override user input param of sha#123

Open
jantiebot wants to merge 2 commits intoadvanced-security:mainfrom
jantiebot:fix/sha-override
Open

fix: correctly override user input param of sha#123
jantiebot wants to merge 2 commits intoadvanced-security:mainfrom
jantiebot:fix/sha-override

Conversation

@jantiebot
Copy link
Copy Markdown

@jantiebot jantiebot commented Mar 30, 2026

For me this fixes the issue where the snapshot-sha input parameter was ignored and instead the head SHA from the github.event.pull_request context was used. Please check if there is a similar issue when using the snapshot-ref parameter. If I recall correctly I couldn't make it work with that input parameter either.

In order to submit the dependency graph of the base branch in a PR, I'm calling this action as follows:

- name: Submit Maven dependency snapshot for ${{ github.base_ref }}
  uses: advanced-security/maven-dependency-submission-action@bcf63663dc640859ca1b6ba4d106a7a73fd53a4c
  with:
    ignore-maven-wrapper: true
    snapshot-sha: ${{ github.event.pull_request.base.sha }}

Now the snapshot-sha input parameter is used properly and the dependency graph is available as expected.

@jantiebot jantiebot requested a review from a team as a code owner March 30, 2026 15:32
Copilot AI review requested due to automatic review settings March 30, 2026 15:32
Copy link
Copy Markdown
Contributor

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

Fixes snapshot SHA overriding so that user-provided snapshotConfig.sha properly takes precedence when generating dependency snapshots (as used by the action/CLI), and updates the compiled dist bundle accordingly.

Changes:

  • Use snapshotConfig.sha (not the newly-created snapshot.sha) when applying a user-specified SHA override.
  • Regenerate dist/index.js, reflecting the SHA override fix plus bundled dependency code updates.

Reviewed changes

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

File Description
src/snapshot-generator.ts Corrects SHA override source to use user-provided snapshotConfig.sha.
dist/index.js Rebuilt bundle reflecting the SHA override change plus additional bundled dependency/version/code updates.

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

Comment on lines +46 to 49
const specifiedSha = getNonEmptyValue(snapshotConfig?.sha);
if (specifiedSha) {
snapshot.sha = specifiedSha;
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

getNonEmptyValue expects a string and calls .trim(), but SnapshotConfig.sha is typed as any. Now that snapshotConfig.sha is being passed in, a non-string value would throw at runtime. Consider tightening SnapshotConfig (sha/ref as string | undefined) and/or making getNonEmptyValue guard on typeof str === 'string' before trimming.

Copilot uses AI. Check for mistakes.
Comment on lines +46 to 49
const specifiedSha = getNonEmptyValue(snapshotConfig?.sha);
if (specifiedSha) {
snapshot.sha = specifiedSha;
}
Copy link

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

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

There are existing tests for generateSnapshot, but this change to override snapshot.sha from snapshotConfig.sha isn’t covered. Add a test that passes a snapshotConfig with sha set and asserts the returned snapshot has that sha (and ideally that whitespace-only input is ignored).

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

2 participants