Skip to content

feat(565): update dir and file input to support working dir#557

Merged
jakobmoellerdev merged 8 commits into
open-component-model:mainfrom
matthiasbruns:feat/565_relative_file_pointers_input
Aug 18, 2025
Merged

feat(565): update dir and file input to support working dir#557
jakobmoellerdev merged 8 commits into
open-component-model:mainfrom
matthiasbruns:feat/565_relative_file_pointers_input

Conversation

@matthiasbruns

@matthiasbruns matthiasbruns commented Aug 13, 2025

Copy link
Copy Markdown
Contributor

Cherry picking constructor changes from #540

What this PR does / why we need it

Resolve file/dir inputs relative to the constructor file by wiring a filesystem working directory through input methods; default-deny path escapes with an explicit opt-out.

Changes (high level):

  • Added WorkingDirectory handling to the input bindings

Which issue(s) this PR fixes

Fixes #565

Copilot AI review requested due to automatic review settings August 13, 2025 14:33
@matthiasbruns matthiasbruns requested a review from a team as a code owner August 13, 2025 14:33
@matthiasbruns matthiasbruns marked this pull request as draft August 13, 2025 14:33
@github-actions github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/m Medium labels Aug 13, 2025
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch 3 times, most recently from f4a2d4a to aad1cd3 Compare August 13, 2025 14:38
@matthiasbruns matthiasbruns marked this pull request as ready for review August 13, 2025 14:39
@jakobmoellerdev jakobmoellerdev changed the title feat(565): update input to support working dir feat(565): update dir and file input to support working dir Aug 13, 2025
jakobmoellerdev
jakobmoellerdev requested changes Aug 13, 2025
View reviewed changes
Comment thread bindings/go/input/dir/method.go Outdated
Comment thread bindings/go/input/file/method.go Outdated
Comment thread bindings/go/input/file/method.go Outdated
Comment thread bindings/go/input/dir/method.go Outdated

This comment was marked as outdated.

Sign in to view

@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch 5 times, most recently from 92983b8 to a53e69d Compare August 14, 2025 11:34
fabianburth
fabianburth requested changes Aug 14, 2025
View reviewed changes
Comment thread bindings/go/input/dir/method.go Outdated
Comment thread bindings/go/input/dir/method.go Outdated
fabianburth
fabianburth reviewed Aug 14, 2025
View reviewed changes
Comment thread bindings/go/input/dir/method.go Outdated
@fabianburth

fabianburth commented Aug 14, 2025

Copy link
Copy Markdown
Contributor

Would it make sense to put the ensureWdOrPath function as a public function somewhere, so it can be reused?

@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from 76d04c4 to f2ee3a0 Compare August 14, 2025 13:54
@github-actions github-actions Bot added the size/l Large label Aug 14, 2025
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from f2ee3a0 to 7857bf7 Compare August 14, 2025 14:16
@matthiasbruns

matthiasbruns commented Aug 14, 2025

Copy link
Copy Markdown
Contributor Author

Would it make sense to put the ensureWdOrPath function as a public function somewhere, so it can be reused?

I askes @jakobmoellerdev where this place would be - maybe you can give me a pointer :D

jakobmoellerdev
jakobmoellerdev reviewed Aug 14, 2025
View reviewed changes
Comment thread bindings/go/input/dir/method.go Outdated
Comment thread bindings/go/input/dir/method.go Outdated
Comment thread bindings/go/input/dir/method.go Outdated
Comment thread bindings/go/input/dir/method.go Outdated
@jakobmoellerdev

jakobmoellerdev commented Aug 14, 2025

Copy link
Copy Markdown
Member

for reuse of your function consider something like

func GetBlobInWorkingDirectory(path, workingDir string) (*Blob, error) {
	path, err := ensureWdOrPath(path, workingDir)
	if err != nil {
		return nil, err
	}
	return GetBlobFromOSPath(path)
}

in the blob/filesystem bindings

@matthiasbruns matthiasbruns mentioned this pull request Aug 15, 2025
Merged
@matthiasbruns

matthiasbruns commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

for reuse of your function consider something like

func GetBlobInWorkingDirectory(path, workingDir string) (*Blob, error) {
	path, err := ensureWdOrPath(path, workingDir)
	if err != nil {
		return nil, err
	}
	return GetBlobFromOSPath(path)
}

in the blob/filesystem bindings

#557

After pr #572 557 is merged, I will update this one

@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from 928deaf to f94b2fa Compare August 15, 2025 07:02
@github-actions github-actions Bot added the size/s Small label Aug 15, 2025
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch 3 times, most recently from ddfa7a9 to a681242 Compare August 15, 2025 14:13
fabianburth
fabianburth requested changes Aug 15, 2025
View reviewed changes

@fabianburth fabianburth 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.

I know this was like this before you touched it already, but since the tests make a lot of asserts (require.NoError(t,...)), we might want to think about introducing an r := require.New(t) and to be able to subsequently call r.NoError(...) without having to pass the testing environment.
You can check other tests for reference.

Comment thread bindings/go/input/dir/blob.go
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob.go
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob_test.go Outdated
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from a681242 to 23f61df Compare August 15, 2025 14:47
@matthiasbruns matthiasbruns mentioned this pull request Aug 16, 2025
Closed
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from c2f4751 to 2e57c0d Compare August 18, 2025 08:26
@matthiasbruns
feat: only allow paths to be in working directory
574d0a0 
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

# Conflicts:
#	bindings/go/input/dir/go.mod
#	bindings/go/input/dir/go.sum
#	bindings/go/input/file/go.mod
#	bindings/go/input/file/go.sum
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from 2e57c0d to c18300e Compare August 18, 2025 08:27
@fabianburth fabianburth mentioned this pull request Aug 18, 2025
Closed
4 tasks
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from c18300e to bc28b91 Compare August 18, 2025 09:09
@matthiasbruns
feat: handle path file errors
63b83af 
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from bc28b91 to 63b83af Compare August 18, 2025 09:09
fabianburth
fabianburth requested changes Aug 18, 2025
View reviewed changes
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/file/blob_test.go Outdated
Comment thread bindings/go/input/file/blob_test.go Outdated
Comment thread bindings/go/input/file/blob_test.go Outdated
Comment thread bindings/go/input/file/blob_test.go Outdated
Comment thread bindings/go/input/file/method.go
matthiasbruns and others added 2 commits August 18, 2025 14:40
@matthiasbruns @fabianburth
Update bindings/go/input/file/method.go
ae78260 
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>

Co-authored-by: Fabian Burth <fabian.burth@sap.com>
@matthiasbruns
chore: clean up tests
f545bc8 
Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from 779935d to f545bc8 Compare August 18, 2025 12:40
@matthiasbruns

matthiasbruns commented Aug 18, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

I hope I covered all requests now and we can merge this thing :D @fabianburth

@matthiasbruns matthiasbruns requested a review from Copilot August 18, 2025 12:41
Copilot AI reviewed Aug 18, 2025
View reviewed changes

Copilot AI 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.

Pull Request Overview

This PR adds working directory support to file and directory input methods, allowing path resolution relative to a constructor file while preventing path traversal attacks. The changes enable secure file/directory handling with configurable working directory settings.

  • Added WorkingDirectory field to InputMethod structs in both file and dir packages
  • Introduced NewInputMethod constructors that accept a working directory parameter
  • Updated blob processing functions to accept and use working directory for path resolution

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
bindings/go/input/file/method.go Added WorkingDirectory field and NewInputMethod constructor; updated ProcessResource to use working directory
bindings/go/input/file/blob_test.go Updated test calls to GetV1FileBlob to include working directory parameter
bindings/go/input/file/blob.go Modified GetV1FileBlob to accept working directory and use filesystem.GetBlobInWorkingDirectory
bindings/go/input/dir/method.go Added WorkingDirectory field and NewInputMethod constructor; updated ProcessResource to use working directory
bindings/go/input/dir/blob_test.go Updated test calls to GetV1DirBlob to include working directory parameter
bindings/go/input/dir/blob.go Modified GetV1DirBlob to accept working directory and added path validation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob_test.go Outdated
Comment thread bindings/go/input/dir/blob_test.go Outdated
@matthiasbruns
Update bindings/go/input/dir/blob_test.go
d07f5e1 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com>
@matthiasbruns matthiasbruns force-pushed the feat/565_relative_file_pointers_input branch from 40711b6 to d07f5e1 Compare August 18, 2025 13:16
@jakobmoellerdev jakobmoellerdev enabled auto-merge (squash) August 18, 2025 17:32
@jakobmoellerdev jakobmoellerdev merged commit 258329b into open-component-model:main Aug 18, 2025
19 checks passed
@matthiasbruns matthiasbruns deleted the feat/565_relative_file_pointers_input branch August 19, 2025 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@fabianburth fabianburth fabianburth approved these changes

@jakobmoellerdev jakobmoellerdev jakobmoellerdev approved these changes

Assignees

No one assigned

Labels

kind/feature new feature, enhancement, improvement, extension size/l Large size/m Medium size/s Small

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

make file and dir inputs support relative paths

4 participants

@matthiasbruns @fabianburth @jakobmoellerdev