Skip to content

Add configurable save directory for Autofill-saved entries - #128

Open
AuthenticSm1les wants to merge 1 commit into
pando85:mainfrom
AuthenticSm1les:feature/save-root-directory
Open

Add configurable save directory for Autofill-saved entries#128
AuthenticSm1les wants to merge 1 commit into
pando85:mainfrom
AuthenticSm1les:feature/save-root-directory

Conversation

@AuthenticSm1les

Copy link
Copy Markdown

Summary

  • Credentials saved via the Autofill framework (the system "Save to Password Store?" prompt, shown while the app is closed) were always written directly to the repository root, with no way to route them into a category subfolder.
  • Adds a "Save directory" setting under Settings > Autofill (AutofillPreferences.saveDirectory()) that lets users specify a root subfolder (e.g. www); Autofill-originated saves are then resolved under that folder instead of the store root.
  • Scoped entirely to the Autofill save path (AutofillSaveActivity) — leaving it unset preserves existing behavior exactly, and the in-app "+" entry creation flow is unaffected in every case.

Fixes #127

Test plan

  • ./gradlew :app:compileDebugKotlin — builds clean
  • ./gradlew spotlessCheck — formatting passes
  • Manual: set "Save directory" to www, trigger an Autofill save prompt on a new site, confirm the entry lands at www/<site>/<file> instead of the store root
  • Manual: leave "Save directory" empty, confirm Autofill saves still land at the store root as before

Credentials saved via the Autofill framework (the system "Save to
Password Store?" prompt shown while the app is closed) were always
written directly to the repository root, with no way to route them
into a category subfolder like a user-organized store might use.

Add a "Save directory" setting under Settings > Autofill that lets
users specify a root subfolder; Autofill-originated saves are then
resolved under that folder instead of the store root. Leaving it
unset preserves the existing behavior exactly. The in-app "+" entry
creation flow is unaffected in every case.

Fixes pando85#127

@pando85 pando85 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks for this. I like the feature and the current Autofill-only scope, and I'm happy to merge it after a couple of changes.

There are two things I'd like to address before merging:

  1. Validate the configured path instead of silently rewriting it

    Currently saveDirectory() removes blank and .. path segments:

    .split('/')
    .filter { it.isNotBlank() && it != ".." }
    .joinToString("/")

    This means something like work/../personal silently becomes work/personal, which is safe from straightforward traversal but surprising behavior.

    I'd prefer the configured value to be treated as a relative path and rejected/fallback safely when invalid, rather than mutated into a different path.

    Please cover at least:

    • empty value → repository root
    • www → valid
    • www/personal → valid
    • /www → invalid
    • ../www → invalid
    • www/../personal → invalid
    • . / empty path components → invalid or explicitly normalized

    It would also be cleaner if AutofillPreferences only returned the configured relative directory rather than depending on PasswordRepository; the caller already has the repository root and can resolve it there. I don't consider that architectural cleanup a hard requirement if path handling is otherwise robust.

  2. Add unit tests for the path handling

    Since the new behavior is mostly path parsing/resolution, I'd like this logic covered by unit tests rather than relying only on compilation and manual verification.

After that, please also confirm the two manual cases from the PR description:

  • configured directory: an Autofill save actually lands below that directory
  • empty setting: existing root-save behavior is preserved

No need to expand this PR to change the normal in-app + flow or add a folder picker. Keeping this focused on Autofill is good.

With those changes, this should be ready to merge. Thanks!

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.

[FEATURE] Configurable root/category prefix for Autofill-saved entries

2 participants