FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Stop setting `CODEQL_RUNNER` environment variable if CLI already sets it by angelapwen · Pull Request #2081 · github/codeql-action · GitHub

Stop setting CODEQL_RUNNER environment variable if CLI already sets it - #2081

Merged
angelapwen merged 4 commits into
github:mainfrom
angelapwen:runner-env-var-feature
Jan 12, 2024
Merged

Stop setting CODEQL_RUNNER environment variable if CLI already sets it#2081
angelapwen merged 4 commits into
github:mainfrom
angelapwen:runner-env-var-feature

Conversation

angelapwen commented Jan 11, 2024
edited
Loading

Copy link
Copy Markdown
Contributor

We are now setting the CODEQL_RUNNER environment variable in the codeql database init step (see backlinked internal PR), so for versions of the CLI that support this feature, we can stop setting it in the Action.

Merge / deployment checklist

  • Confirm this change is backwards compatible with existing workflows.
  • Confirm the readme has been updated if necessary.
  • Confirm the changelog has been updated if necessary.

Copy link
Copy Markdown
Contributor Author

Hm...

Deleting failed SARIF upload
  In test mode, therefore deleting the failed analysis to avoid impacting tool status for the Action repository. SARIF ID to delete: 3146eec2-b0d6-11ee-9ed6-a261d13cdc7d.
  Analysis ID to delete: 160977804.
Error: Failed to delete uploaded SARIF analysis. Reason: Error: HttpError: Resource not accessible by integration

Odd as we have not changed anything about our token permissions between this PR and the last run. Also, our PUT calls to the analysis and analysis/status endpoints are working, so we should still have the necessary security_events:write permissions for a DELETE (https://docs.github.com/en/rest/code-scanning/code-scanning?apiVersion=2022-11-28#delete-a-code-scanning-analysis-from-a-repository)

angelapwen marked this pull request as ready for review January 11, 2024 23:42
angelapwen requested a review from a team as a code owner January 11, 2024 23:42

aeisenberg commented Jan 12, 2024
edited
Loading

Copy link
Copy Markdown
Contributor

That is odd. Maybe try re-running just in case that was a fluke. The workflow has security-events: write

Copy link
Copy Markdown
Contributor Author

I had re-run it on a previous commit too, probably have 5 total consecutive runs failing now 🤔 might be some kind of API error but odd that it's not hit on any of the other times we're hitting code scanning endpoints.

Copy link
Copy Markdown
Contributor

I think the "Submit SARIF after failure" check doesn't work on forks — we need to update it so it doesn't try to delete the SARIF when running from a fork. I've backlinked an internal issue.

Copy link
Copy Markdown
Contributor

Right... I forgot about that.

aeisenberg left a comment

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.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

Copy link
Copy Markdown
Contributor Author

I think the "Submit SARIF after failure" check doesn't work on forks — we need to update it so it doesn't try to delete the SARIF when running from a fork. I've backlinked an internal issue.

Oh shoot I also forgot about that. Thanks!

Copy link
Copy Markdown
Contributor Author

I'm going to remove it from being Required until we have that backlinked issue done.

angelapwen merged commit 9653106 into github:main Jan 12, 2024
angelapwen deleted the runner-env-var-feature branch January 12, 2024 17:41
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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


Back | FazBrowse Home | New Git URL