Skip to content

Conversation

@watson81
Copy link

Summary

This PR includes several fixes and improvements for exporting attachments. This includes:

  • Ensuring that exported attachment paths include only file-system-safe characters, preventing mysterious error messages and failures to export
  • Correcting a bug in lpass-att-export.sh that caused attachment file names from being truncated when they contain a space.
  • Changing a character class string to be POSIX compliant, enabling lpass-att-export.sh to be used with MacOS' default sed.
  • Improving the speed of exporting attachments by lpass-att-export.sh, especially when a vault includes items with master password reprompt enabled
  • Resolving shellcheck warnings in lpass-att-export.sh. This was mostly just implementing shell best practices, but does resolve one bug also.

Details

File-System-Safety

Share and Group names are arbitrary strings and could contain characters that are not valid on file systems such as forward slashes ('/') and theoretically things like control characters. This applies to attachment file names also, though less commonly so since they usually will have a name from the uploading user's file system.

Thus, when exporting attachments, these strings must be sanitized. This is painful to do in lpass-att-export.sh, so this PR introduces the new '_' format string modifier. This modifier works similarly to the existing '/' modifier, but causes instances of forward slashes, carriage returns, new lines, and tabs with an underscore in output.

lpass-att-export.sh has been modified to use this new format string when a new-enough version of lpass is present.

  • I guessed that this PR will be released in a new v1.7.0 release. Please adjust version number parsing in lpass-att-export.sh if a different release number is selected.

Truncated Attachment Names

lpass-att-export.sh used awk to read space separated tokens from the attachment description printed by lpass. This works fine for the attachment id, but incorrectly truncates file names containing spaces. Using the bash built-in read not only solves this but also prevents needing 2 external program calls and a temp variable.

POSIX compliant sed

The version of sed that currently ships with MacOS is old and only supports POSIX regular expressions. Thus, it supports the [:space:] character class but not the shorthand /s escape-like character class. So we switch to the long version and everyone is happy.

Export Speed

lpass-att-export.sh used lpass ls to iterate through every item in the vault, calling lpass show for each in order to find any that contain attachments. This will prompt for your master password for every item with a reprompt set, whether they have an attachment or not. It also is a lot of calls to PBKDF2 and if your interation count is high enough, the computation time can add up.

Instead we now use lpass export to find the ids of items that contain attachments via the id and attachpresent fields. This doesn't reprompt for every item. Much faster and less annoying!

Shellcheck Warnings

Most of the changes here were just to add quotes to ensure that variables don't get parsed wrong, and in most cases this doesn't actually change anything. But the script currently sometimes exits with a failure exit code even when it works fine due to the let attcount-=1 at the very end. Switching to (( attcount-=1 )) || true prevents this. Not that it's a really big deal, but might as well fix it.

The MacOS version of sed doesn't support the /s character class. It does
support [:space:] though.

Signed-off-by: Patrick Watson <watson81@users.noreply.github.com>
Signed-off-by: Patrick Watson <watson81@users.noreply.github.com>
Use an export to find the ids of item that may contain attachments. This
is better than showing every item since showing every item will prompt
for your master password for every item with a reprompt set instead of
only those we need to actually export. It will also be significantly
faster on older hardware or when the master password PBKDF2 iterations
are large.

Signed-off-by: Patrick Watson <watson81@users.noreply.github.com>
lpass-att-export.sh used awk to read space separated tokens from the
attachment description. This works fine for the id, but incorrectly
truncats file names containing spaces. Using read not only solves this
but also prevents needing 2 external program calls and a temp variable.

Signed-off-by: Patrick Watson <watson81@users.noreply.github.com>
Share and Group names are arbitrary strings and could contain characters
that are not valid on file systems such as forward slashes ('/') or
control characters. This applies to attachment file names also, though
less commonly so since they usually will have a name from the uploading
user's file system.

Thus, when exporting attachments, these strings must be sanitized. This
is painful to do in lpass-att-export.sh, so this commit introduces the
new '_' format string modifier. This modifier works similarly to the
existing '/' modifier, but causes instances of forward slashes, carriage
returns, new lines, and tabs with an underscore in output.

lpass-att-export.sh has been modified to use this new format string when
a new-enough version of lpass is present.

Signed-off-by: Patrick Watson <watson81@users.noreply.github.com>
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.

1 participant