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

staticrypt not rendering in-place as expected #196

Open
bmcminn opened this issue Mar 15, 2024 · 2 comments · May be fixed by #200
Open

staticrypt not rendering in-place as expected #196

bmcminn opened this issue Mar 15, 2024 · 2 comments · May be fixed by #200
Labels

Comments

@bmcminn
Copy link

bmcminn commented Mar 15, 2024

Attempting to run staticrypt via npx and the resulting file is not rendering in-place as I expect it to

What's happening

I enter the command:

npx staticrypt --short ./deploy/encrypted/index.html --password testing

I'm just attempting to verify the utility works as expected so the short password is fine for now, however the resulting file ends up being written to disk at ./encrypted/index.html instead of ./deploy/encrypted/index.html and this makes zero sense to me.

Digging into the source code this appears to be an unintended pathname collision with the default --directory value "encrypted" assigned in the parseCommandLineArguments() method in helpers.js

What should be happening

I would expect this utility to work in-place and render the target asset where it was previously defined, meaning any fully qualified filepath I pass is mapped implicitly, and the --directory value is a strict output path override that is only used if I provide a value to it.

Justification

I'm using this to encrypt statically generated files for my website that already exist in my ./deploy folder, so I'm not worried about accidentally overwriting my source file as that is not the desired target.

I can see a need for providing a confirmation prompt to the user that this change will be handled in-place and will overwrite the original source file.

Optionally, to avoid any breaking changes with the existing behavior, it would be great to see a --overwrite flag that tells staticrypt to overwrite the files in-place as I expect, and at least this way it's a more obvious and intentional behavior than expecting the user to know they should provide their own --directory argument.

My setup

StatiCrypt version: 3.4.0
Node version: 21.7.1
OS: Ubuntu Linux 22.04

Sample HTML

<!DOCTYPE html>
<html lang="en">
<head>
	<meta charset="UTF-8">
	<meta name="viewport" content="width=device-width, initial-scale=1.0">
	<title>Document</title>
</head>
<body>
	Testing Staticrypt
</body>
</html>
@bmcminn bmcminn added the bug label Mar 15, 2024
@robinmoisson
Copy link
Owner

robinmoisson commented Apr 17, 2024

Hi @bmcminn,

Thanks for opening the discussion on that topic. Sorry about the late reply. Are you doing the encryption as part of a build step?

It looks like it's working as intended to me: I want to avoid the default being overwriting the original files so that there's not risk of data loss. Earlier versions of staticrypt would create test_encrypted.html alongside test.html but as you might imagine that didn't scale well to multi-page projects, so we moved to having a whole new directory created.

This makes it easy to encrypt a full directory - all non-html files (css, js, etc) in the input directory are copied to the new directory and should be present in the encrypted/ directory, so in a build script it would be easy to do something like

# encrypt to `encrypted/` with something like `npx staticrypt -r deploy`
rm -rf deploy
mv encrypted deploy

Or if you want to replace all files in place in deploy/encrypted you could do as mentioned here and do

npx staticrypt deploy/encrypted/* -r -d deploy/encrypted

(or maybe cd deploy first)

This makes things a bit heavy when you want to just encrypt one file (your output it in a whole other directory) and I've been meaning to add a -o my_encrypted_file.html option to give more granularity for a while now, but I feel that's not really what you're talking about here.

@bmcminn
Copy link
Author

bmcminn commented Apr 17, 2024

For my use-case I'm using a password flag in the frontmatter data of my content files to indicate to my build process that this document should be encrypted with the assigned pass phrase.

In tinkering with this utility I got it working just fine now. However, as you mentioned, the way this utility is designed makes being granular a bit challenging.

The main thing that was unintuitive is the --directory flag is at play at all times and is the defacto output directory. Suppose reading the docs more closely would have made that more apparent, but combined with the fact my test directory was named encrypted the same as --directory defaulting to ./encrypted made it feel like a wierd bug. So this may be a need for clearer documentation or more samples to showcase how the utility is used.

I do think an --overwrite flag could be useful, or perhaps a set of explicit --source and --output path mapping arguments could be a more intuitive pattern.

Additionally this could be trivialized by the JS API suggestion I made in #195

vuyp added a commit to vuyp/staticrypt that referenced this issue Sep 3, 2024
Fixes robinmoisson#196

Add support for the `--overwrite` flag to overwrite files in-place.

* **cli/helpers.js**
  - Add `overwrite` parameter to `getFullOutputPath` and `recursivelyApplyCallbackToHtmlFiles` functions.
  - Update `parseCommandLineArguments` to include the `--overwrite` option.

* **cli/index.js**
  - Update `runStatiCrypt` function to handle the `--overwrite` flag.
  - Update `encodeAndGenerateFile` function to handle the `--overwrite` flag.

* **README.md**
  - Add documentation for the `--overwrite` flag in the CLI section.
  - Add an example of using the `--overwrite` flag.
@vuyp vuyp linked a pull request Sep 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants