| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
Warning
Fixes shorthand remote configuration addresses being mistaken for local paths.
Changes:
| File | Description |
|---|---|
| src/config/remote-file.ts | Extracts new-format address parsing. |
| src/config-utils.ts | Updates configuration source selection. |
| src/config-utils.test.ts | Adds loading behavior tests. |
| lib/entry-points.js | Generated artifact; excluded from review. |
Sorry, something went wrong.
| // If the path is relative to the workspace and the file exists, then we use it. | ||
| if (workspaceRelative && fs.existsSync(localFilePath)) { | ||
| configFile = localFilePath; | ||
| localFile = true; |
There was a problem hiding this comment.
For backwards-compatibility reasons, we cannot rely on local paths starting with ./. Adding such a requirement would break existing configurations.
That said, I am also not convinced that this is a real concern. While an analysis may end up using a local configuration file in a PR branch, rather than a remote one that was intended, this would be very obvious.
Currently, the same thing could be done in any repository that uses a custom workflow by changing the configuration file in the repo, adding one and setting config-file, or setting the config input.
Sorry, something went wrong.
| // If the FF for the new format is not enabled or the input path is explicitly local, | ||
| // throw the old errors depending on whether the file is outside of the workspace or not. | ||
| // Otherwise, we assume the path refers to a remote file. | ||
| if (isRelativePath(configFile) || !allowNewFormat) { |
There was a problem hiding this comment.
I don't believe that input paths are expected to be absolute or that it is really supported since paths have to be relative to the workspace root.
Sorry, something went wrong.
|
I'll close this for now in favour of #4014. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
The loadUserConfig function uses isLocal to determine if a provided configFile path is local or remote. The logic in isLocal is based on the OLD_REMOTE_ADDRESS_FORMAT where all components are required and the presence of an @ character is used as an indicator that the address refers to a remote file.
With the changes from #3973, the new remote address format makes all components, except the repository name, optional. Therefore, a new-style remote address may not contain an @ character. I added a test case which demonstrates the resulting failure in 3ac4070.
Working around this issue is slightly complicated, because it is (in general) impossible to distinguish between a relative, local path like foo.yml and a repository name (foo.yml is also a valid repository name).
The approach I have taken here to resolve this is principally that:
This has the effect that:
Risk assessment
For internal use only. Please select the risk level of this change:
Which use cases does this change impact?
Workflow types:
Products:
Environments:
How did/will you validate this change?
If something goes wrong after this change is released, what are the mitigation and rollback strategies?
How will you know if something goes wrong after this change is released?
Are there any special considerations for merging or releasing this change?
Merge / deployment checklist