-
Notifications
You must be signed in to change notification settings - Fork 204
Prevent local absolute paths from being entered in App config #2814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Code coverage for golang is
|
|
|
||
| func (r *Reporter) readApplicationInfo(ctx context.Context, cfgFilePath string) (*model.ApplicationInfo, error) { | ||
| b, err := fs.ReadFile(r.fileSystem, cfgFilePath) | ||
| func (r *Reporter) readApplicationInfo(appPath, cfgFileAbsPath string) (*model.ApplicationInfo, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or appDirPath, cfgFileRelPath
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, it should be repoDir, appDirRelPath, cfgFilename
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean we should use the same thing in our code and data in db, they are appPath and cfgFilename, and repoDir is added for dealing with the local file system.
Co-authored-by: Le Van Nghia <[email protected]>
|
@nghialv Applied! PTAL |
|
Looks nice. |
|
Code coverage for golang is
|
|
Nice |
What this PR does / why we need it:
This PR solves the problem where Piped would specify a local absolute path as GitPath when reporting a Git-defined Config to Control-plane.
Which issue(s) this PR fixes:
Close #2806
Does this PR introduce a user-facing change?: