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

[BUG] $:/config/OriginalTiddlerPaths - paths outside the wiki folder are encoded. #5483

Closed
joshuafontany opened this issue Feb 8, 2021 · 9 comments · Fixed by #5504
Closed

Comments

@joshuafontany
Copy link
Contributor

joshuafontany commented Feb 8, 2021

Describe the bug
Over on GoogleGroups, there has been some confusion on how to use the new isEditableFile feature when importing files external to the Wiki directory. There is also a bug that encodes these filepaths, instead of using the "original" relative path found in $:/config/OriginalTiddlerPaths.

To Reproduce
Steps to reproduce the behavior:

  1. Setup a node wiki folder as given in https://tiddlywiki.com/#tiddlywiki.files%20Files and https://tiddlywiki.com/#Customising%20Tiddler%20File%20Naming, with tiddlers being loaded from OUTSIDE the Wiki directory.
  2. Edit and save an "external tiddler".

Observed behavior
The tiddler is saved to the Wiki's tiddlers/ folder with an encoded filename if the tiddler was read from a file outside the Wiki directory.

Expected behavior
The tiddler is saved back to its "original" file location.

Here we have a security concern. If we only rely on $:/config/OriginalTiddlerPaths to store the "original" location and a user is able to change the contents of $:/config/OriginalTiddlerPaths, then they can acquire write access to anywhere on-disk.

My first instinct is to store and load that info in the internal $tw variables, and use $:/config/OriginalTiddlerPaths only as a read-only log of the files loaded and their relative locations to the Wiki directory. Then remove the tiddlers loaded with the isEditableFile from any of the $:/config/FileSystem* filters. If we enforce using the original filepath and extension with no way of editing it, that would minimize the security concerns. If the user would like that tiddler's data to be subject to the $:/config/FileSystem* filters, they can re-name the tiddler to a title that does not appear in $:/config/OriginalTiddlerPaths.

I think this will be important to implement before we merge in the PR(#5275) that will introduce the searchSubdirectories flag, as that will generate a lot more tiddlers that may have isEditableFile set...

Working on a PR now.

@joshuafontany
Copy link
Contributor Author

joshuafontany commented Feb 8, 2021

@saqimtiaz It would be great to get your input on this, I know you had a special case where, I think, you were loading .txt files as type: text/vnd.tiddlywiki tiddlers, but wanted to save them as *.txt and *.txt.meta files, right? Isn't that why we pass isEditableTiddler files to the $:/config/FileSystem* filters?

I'm really leaning towards "freezing" the filepath for tiddlers loaded via isEditableFile, and only allowing the original extension or .json (for cases when the meta-data as "unsafe fields").

@Jermolene @pmario I think you also use the Node server the most. What do you all think about this one?

@rubaboo
Copy link
Contributor

rubaboo commented Feb 8, 2021

Huh. I always assumed $:/config/OriginalTiddlerPaths was FYI and did not matter once TW is loaded.

Google Groups discussion is here.

@saqimtiaz
Copy link
Member

I am not convinced that the bug described in this issue actually exists, I am not able to reproduce it on 5.1.23.

Can you please provide step my step directions for reproducing the bug on 5.1.23, or alternatively a zipped directory structure that can be used for recreating the problem?

Or is this problem only present in the pre-release? If so, it is clearly a regression since the last release, that we need to fix without removing other features. One potential approach here could be to limit the use of the isEditableFile feature to files loaded from within the wiki directory, though I haven't looked at the code in some time so I am unsure how easy that would be to enforce.

I'm really leaning towards "freezing" the filepath for tiddlers loaded via isEditableFile, and only allowing the original extension or .json (for cases when the meta-data as "unsafe fields").

@joshuafontany that would not be backwards compatible with the changes introduced in 5.1.23, or even the workflow I had in 5.1.22, and would indeed be a breaking change that significantly hamper my ability to continue using TiddlyWiki.

Freezing the filepath for tiddlers loaded with isEditableFile would mean they cannot be renamed without resulting in duplicate tiddlers, avoiding which was the reason for introducing the isEditableFile flag in the first place.

@rubaboo
Copy link
Contributor

rubaboo commented Feb 8, 2021

Yes, this problem is only present in the pre-release.

@rubaboo
Copy link
Contributor

rubaboo commented Feb 8, 2021

to limit the use of the isEditableFile feature to files loaded from within the wiki directory

would be a bummer.

I don't know the internals involved here, but from end-user perspective, I'd like to save external files back to their location outside wiki folder, but keep *.meta files in the wiki folder, maybe even have a line for them in $:/config/FileSystemPaths. The idea here is to keep my scripts library "clean" of extraneous files. Metadata that only TW understands and makes use of belongs with the wiki, not with my collection of scripts.

@joshuafontany
Copy link
Contributor Author

@saqimtiaz Thank you, it was the duplicate tiddlers on edit/server-restart that was the issue I was not remembering clearly. That really helps to narrow this regression down (& yes it was confirmed on GG to be specific to 5.1.24-pre).

@rubaboo I agree that being able to load/re-save files from anywhere on disk, and then keep the *.meta files in the Wiki folder would be ideal... I'll have to think on the logic around that while I target a fix for this regression. Thanks!

@rubaboo
Copy link
Contributor

rubaboo commented Feb 9, 2021

EDIT: I'll open a new issue for the problem I'm having with file system extensions.

@joshuafontany
Copy link
Contributor Author

joshuafontany commented Feb 18, 2021

PR for the base bug up @ #5504.

I will consider the feature-request of storing the *.meta files in the wiki folder if the base file is not being written to the wiki folder. That should be worked through with the changes in PR #5275 which adds the searchSubdirectories flag to tiddlywiki.files config files. I will have to go back and review that code in light of recent changes.

Best,
Joshua F

@saqimtiaz
Copy link
Member

I will consider the feature-request of storing the *.meta files in the wiki folder if the base file is not being written to the wiki folder.

@joshuafontany I'm a bit concerned that this could get hairy pretty fast. We would need to account for tiddlers that might be solved in different directories but with the same file name, trying to save their meta file in the same folder. Just something to keep in mind. It seems a lot easier to just ignore the .meta files for non-TW workflows in the directory the tiddlers are stored.

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 a pull request may close this issue.

3 participants