Skip to content

fix: never regenerate a linked .env that could not be read - #5

Open
ilmakio wants to merge 1 commit into
mainfrom
fix/env-write-fidelity
Open

ilmakio wants to merge 1 commit into
mainfrom
fix/env-write-fidelity

Conversation

@ilmakio

@ilmakio ilmakio commented Aug 17, 2026

Copy link
Copy Markdown
Owner

Why

Writing an item back to its linked .env is supposed to update the values it tracks and leave the rest of the file alone. The merge was best-effort: if reading the current contents failed, the write fell through to envContents — the document regenerated from the item's fields — and put that on disk, deleting every comment, blank line and untracked variable in it.

That path is reachable, not theoretical: a file holding a single non-UTF-8 byte fails to read as text and writes perfectly well.

Two smaller things in the same area: the write precondition was the digest from the last sync rather than the text that was actually merged, so a change arriving between the read and the rename could be silently replaced; and every tracked assignment was rebuilt double-quoted, so LOG_LEVEL=info came back as LOG_LEVEL="info" and one Write showed up as a diff on every line of a file somebody else maintains.

What changed

  • A parsed link refuses the write when the current contents cannot be read, with a message naming the file.
  • The write precondition is the merged text. Only an explicit overwrite of a file that could not be read at all is left unguarded — there is nothing to compare against in that case.
  • Pushing over a file that moved since the last sync is refused explicitly, instead of relying on the digest check inside the write.
  • Nothing to change means nothing is written: no new modification date, no new inode.
  • A line whose value did not change stays byte-identical. A changed value goes back in the quoting the file used (bare, single, double) whenever that is still safe for the new value; anything a shell or dotenv reader could interpret still falls through to the escaped double-quoted form.
  • A successful write no longer leaves a "this item has changed" badge behind: the vault is compared against the vault, not against the file digest a merged file never matches.
  • The overwrite confirmation now describes what a write to a parsed link actually does.

Tests

PassStoreTests — new coverage for: a write refused on an unreadable file (and the file's bytes left untouched), an accepted overwrite merging into the current on-disk text and keeping a comment and a variable added there afterwards, a no-op write leaving the modification date alone, byte-identical output when no value changed, quote-style preservation, and quoting still winning for unsafe values.

Writing back to a linked file merges into the file on disk, but the merge
was best-effort: when the read failed the write fell through to the
document regenerated from the item's fields, which deletes every comment,
blank line and untracked variable in it. A .env holding one non-UTF-8 byte
reads as unreadable and writes fine, so that path was reachable.

- Refuse the write for a parsed link when the current contents cannot be
  read, and say why.
- Use the merged text as the write precondition instead of the last synced
  digest, so a change landing between the read and the rename is refused
  rather than overwritten. Only an explicit overwrite of a file that could
  not be read at all stays unguarded.
- Refuse to push over a file that moved since the last sync explicitly,
  rather than relying on the digest comparison inside the write.
- Skip the write entirely when the merge changes nothing.
- Keep a line the item does not change byte-identical, and put a changed
  value back in the quoting the file used when that is still safe for it.
  Every tracked assignment used to be rebuilt double-quoted, so one Write
  turned into a diff on every line of somebody's file.
- Report a successful write as in sync by comparing the vault against the
  vault; it was measured against the file digest, which a merged file never
  matches.
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