Skip to content
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

Use the host path when current_folder points to the document storage #1007

Merged
merged 2 commits into from
May 10, 2023

Conversation

xhorak
Copy link
Contributor

@xhorak xhorak commented Apr 24, 2023

Addressing issue #982

@ebassi
Copy link
Collaborator

ebassi commented May 2, 2023

Please, conform to the coding style of the rest of the project.

Copy link
Collaborator

@ebassi ebassi left a comment

Choose a reason for hiding this comment

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

Would have been faster, if github had the ability to deal with suggestions involving more than one line.

src/documents.c Outdated Show resolved Hide resolved
src/documents.c Outdated Show resolved Hide resolved
src/documents.c Outdated Show resolved Hide resolved
src/documents.c Outdated Show resolved Hide resolved
src/documents.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
@xhorak
Copy link
Contributor Author

xhorak commented May 2, 2023

Addressed also: xhorak@b3c3f56#r111467974

@xhorak xhorak requested a review from ebassi May 2, 2023 15:49
src/file-chooser.c Outdated Show resolved Hide resolved
@xhorak xhorak requested a review from matthiasclasen May 3, 2023 08:55
g_autoptr(GVariant) value =
g_variant_lookup_value (arg_options, "current_folder", G_VARIANT_TYPE_BYTESTRING);

if (value) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Niggle: move the { to the next line

Copy link
Contributor

Choose a reason for hiding this comment

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

Apart from that, this looks good to me now.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The indentation is wrong too

@jadahl
Copy link
Collaborator

jadahl commented May 3, 2023

The commits should be squashed and turned into the first one before landing.

If the application stores the path of the last saved file for
later save requests to open the same directory it currently points to
/run/user/<uid>/doc/<doc_id> using 'current_folder' option.

The patch uses the <doc_id> to lookup the host path from the document
portal and replaces the current_folder with that.

Resolves: flatpak#982
src/file-chooser.c Outdated Show resolved Hide resolved
src/file-chooser.c Outdated Show resolved Hide resolved
@xhorak xhorak requested a review from ebassi May 5, 2023 10:46
@xhorak
Copy link
Contributor Author

xhorak commented May 10, 2023

@ebassi could we please finish this?

@matthiasclasen
Copy link
Contributor

From my perspective, this is good to go

@ebassi
Copy link
Collaborator

ebassi commented May 10, 2023

Same here, so I guess I get to press the green button. 😄

@ebassi ebassi merged commit 2b92df1 into flatpak:main May 10, 2023
3 checks passed
@lissyx
Copy link

lissyx commented Jun 13, 2023

So while it helps re-open the folder with a "nice name" we still save an XDG document path. Is it possible we have this feature exposed somehow ? Via DBus ? Or maybe it is already doable ? @matthiasclasen @ebassi ?

@ssokolow
Copy link

Isn't unprivileged code always seeing an XDG document path part of the point for ensuring that unnecessary information that may be contained in the names of ancestor folders isn't leaked?

@ebassi
Copy link
Collaborator

ebassi commented Sep 25, 2023

@lissyx Please, open an issue. Commenting on a closed merge request isn't going to help in tracking issues.

@ebassi
Copy link
Collaborator

ebassi commented Sep 25, 2023

@ssokolow The document storage is not for avoiding "leaking" the original path: it's for providing access to a file. There's no expectation of privacy to be "leaked" in the first place.

@lissyx
Copy link

lissyx commented Sep 25, 2023

@lissyx Please, open an issue. Commenting on a closed merge request isn't going to help in tracking issues.

I was asking a question, not reporting a bug

@ssokolow
Copy link

@ssokolow The document storage is not for avoiding "leaking" the original path: it's for providing access to a file. There's no expectation of privacy to be "leaked" in the first place.

In that case, I'm mildly surprised that the FUSE filesystem doesn't just present a grant for /home/you/Documents/foo.pdf as $DOCUMENT_PORTAL_PREFIX/home/you/Documents/foo.pdf with new grants just appearing as new paths within that virtual filesystem.

It'd make it much easier for things like some text editors' $PWD-relative or current-document-relative rendering of other documents' paths to Just Work™ more often.

@voidpointertonull
Copy link

@ssokolow The document storage is not for avoiding "leaking" the original path: it's for providing access to a file. There's no expectation of privacy to be "leaked" in the first place.

So was privacy provided just by coincidence with no plan of supporting this aspect in the future?
It's a quite important detail as many of us started preferring Flatpak due to the shift from just packaging programs to also isolating them which used to be desired mostly to just protect the system, but nowadays it's almost as important to prevent unnecessary data collection done so aggressively that the behavior would have been classified as malware-like earlier.

I'm curious if my expectations are really out of the scope of the project, but what I seek in Flatpak is mostly what I used to seek in browsers which is keeping unknown, often hostile by design programs in check including privacy ("data security") too to some degree.
For example browsers exhibit an I believe decent compromise which is the same what could be observed during the usage of the file access portal: Files are provided with their original name, but no path information gets leaked to the target as the path is meaningless to the vast majority of benign targets while it possibly contains sensitive information useful for malicious purposes.

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.

None yet

7 participants