Skip to content

Rely on os.Root for checking paths #2752

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

Merged
merged 3 commits into from
Jul 22, 2025

Conversation

jsoriano
Copy link
Member

Fix #2751.

Remove pathIsInRepositoryRoot, and rely on implicit os.Root operations to check if a path is under the repository root.

@jsoriano jsoriano requested a review from a team July 21, 2025 16:30
@jsoriano jsoriano self-assigned this Jul 21, 2025
@@ -845,13 +855,13 @@ func TestLinksFS_WorkDirValidation(t *testing.T) {
name: "invalid absolute workDir outside repo",
workDir: outsideDir,
expectError: true,
errorMsg: "is outside the repository root",
errorMsg: "path escapes from parent",
Copy link
Member Author

Choose a reason for hiding this comment

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

Error from the stdlib is directly returned now, but we cannot check for it to convert it as it is not public (https://github.com/golang/go/blob/e5502e0959bb54ec70ca500e8d2b6f5ac5efbc53/src/os/file.go#L421).

We could check the error message and replace it, but not sure if it is worth it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it could be kept like this for now.

Comment on lines 788 to 793
errorMsg: "escapes the repository root",
errorMsg: "no such file or directory",
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the issue solved by this PR.

@@ -845,13 +855,13 @@ func TestLinksFS_WorkDirValidation(t *testing.T) {
name: "invalid absolute workDir outside repo",
workDir: outsideDir,
expectError: true,
errorMsg: "is outside the repository root",
errorMsg: "path escapes from parent",
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it could be kept like this for now.

@jsoriano jsoriano marked this pull request as ready for review July 21, 2025 19:25
@jsoriano jsoriano requested a review from mrodm July 21, 2025 19:25
@jsoriano
Copy link
Member Author

test integrations

@elastic-vault-github-plugin-prod

Created or updated PR in integrations repository to test this version. Check elastic/integrations#14614

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cc @jsoriano

@jsoriano jsoriano requested a review from marc-gr July 21, 2025 20:40
Copy link
Contributor

@mrodm mrodm left a comment

Choose a reason for hiding this comment

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

LGTM

@jsoriano jsoriano merged commit 2f30958 into elastic:main Jul 22, 2025
3 checks passed
@jsoriano jsoriano deleted the fix-links-error-message branch July 22, 2025 14:20
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.

Fix error message when file linked is not found
3 participants