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

hledger-web: actually add transaction to the selected journal file #1873

Closed

Conversation

erictapen
Copy link

@erictapen erictapen commented Jun 16, 2022

[Fix for #1229]

The hledger-web interface currently allows for selecting a file on which to append a transaction, but currently that data isn't used anywhere. It just writes to the default file, regardless of the selection.

tmp CpdrFTokbK

My solution is somewhat of a hack; I use the tsourcepos metadata in the Transaction type to carry the preffered journal file around.

Also this currently has the security implication, that it allows users to append transactions to arbitrary files. It probably would make sense to only restrict this to the files that are discovered by hledger, but I'm not familiar with the logic.

@simonmichael
Copy link
Owner

it allows users to append transactions to arbitrary files

Thanks for the PR and for calling out the above, which does sound like a blocker.

@simonmichael simonmichael added the web The hledger-web tool. label Jul 31, 2022
@crocodile-code-review
Copy link

Review on Crocodile

@erictapen
Copy link
Author

Thanks for the PR and for calling out the above, which does sound like a blocker.

Do you have any pointer to where I could get a list of allowed files from? I'm a bit lost in the code base currently.

@simonmichael
Copy link
Owner

Sure, hledger-lib:Hledger.Journal.Data.journalFilePaths will list all the journal's files.

@simonmichael
Copy link
Owner

PS, this worked once - possible some spelunking in code history is useful ?

@erictapen
Copy link
Author

erictapen commented Aug 20, 2022

I implemented the check for the journal file. Didn't look at the code history though, so I have no idea what broke this.

I also quickly tested this on my setup and it seems to work. Adding to legitimate files still works, but forging the file path to a different one produces the desired error message without appending to the file.

@simonmichael
Copy link
Owner

We went with a different fix, see #1229.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
web The hledger-web tool.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants