Create remote file links to GitHub URL#1209
Conversation
|
Old query history items (i.e. whose result index didn't contain |
adityasharad
left a comment
adityasharad
left a comment
There was a problem hiding this comment.
This looks neat, and the wiring of the new links generally looks good. I have one suggestion on how to resolve non-SARIF locations, which can be fiddly -- let me know if you want to talk through the details separately.
| // Remote locations have the following format: | ||
| // file:/home/runner/work/<repo>/<repo/relative/path/to/file | ||
| // So we need to drop the first 6 parts of the path. | ||
| const locationParts = resolvableLocation.uri.split('/'); | ||
| const trimmedLocation = locationParts.slice(6, locationParts.length).join('/'); |
There was a problem hiding this comment.
This is a clever trick, and is good enough as a starting point for most runs right now. Unfortunately in the long run, I suspect there is a risk of this breaking quietly because it depends on the exact filesystem layout during the workflow run that originally created the database. Here are some examples I could think of where the assumptions will not hold:
- the run might be on Windows, so you have something like
C:\runner\work\<nwo>as the prefix (likely) - the run might have checked out the repo to a different folder instead of the root of the workspace, in which case the prefix is
/home/runner/work/<nwo>/differentFolder(somewhat likely) - the run might be on a self-hosted runner that doesn't follow the
/home/runner/work/<nwo>convention (unlikely but possible)
Assuming we have to handle these cases, what should we do? Fortunately I think there is some logic from the local queries part of the extension that we can learn from, and a useful field populated by the CLI. CodeQL databases have a sourceLocationPrefix field in codeql-database.yml that describes the path prefix to the source code (/home/runner/work/<nwo>/ in your example). Have you got that data already from the remote run?
Once you have that, you can remove the sourceLocationPrefix from the beginning of the resolvableLocation.uri, and what remains will be the relative path you're looking for.
(The key difference between local and remote queries here is that local queries have to resolve the location against the source archive on the local filesystem, while remote queries have to resolve the location against GitHub repo URLs. There is the possibility of sharing code between them and making this distinction clear, but it's overkill for this PR.)
There was a problem hiding this comment.
Thank you for the detailed breakdown! 🙇🏽♀️
You're definitely right that this won't be reliable in the long run 🙈 Since we have enough control over the actions runs for the upcoming private beta, this should work for now.
I'll open an issue to fix this properly though 😄 Your suggestion of using the sourceLocationPrefix from the action sounds good! We don't currently have access to that in the extension, but we can download it as part of the metadata (along with things like nwo, sha etc)
3 participants
Follow-up to #1205.
Creates a clickable link in the results view that links to the file location on GitHub. e.g.
links to
https://github.com/meteor/meteor/blob/HEAD/npm-packages/meteor-installer/install.js#L253-L257 (using the exact SHA instead of HEAD, when available).
Checklist
N/A - still internal only 🗳️
ready-for-doc-reviewlabel there.