Decision needed: account credentials committed in test/integration/live-login.ts
#14
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
test/integration/live-login.tscontains, in cleartext in git history:RECOVERY_KEYconstant and thendiscarded with
void RECOVERY_KEY).A comment above them says the account is a throwaway used only for integration testing.
REPO_POLICIES.mdis unambiguous: "Never commit secrets..envfiles, credentials, API keys,and private keys must be in
.gitignore. No exceptions." A recovery mnemonic is the strongestpossible credential for an end-to-end encrypted account — it is the master key. Shipping one
in the repository of a client for that very service is also a bad look for a project whose
entire pitch is that it implements the crypto correctly.
Separately, this file is not a test: it lacks
.test.in its name, so vitest never runs it.It is a manual script that no automated gate exercises.
This needs your decision
Only you can make this call, because it involves an account you own and possibly a history
rewrite.
Option A — rotate and remove, leave history alone (recommended).
Delete the account (or at minimum rotate its password and recovery key), then remove the
constants from the file and read them from environment variables instead, documenting the
variable names in the README. History still contains the old, now-worthless credentials.
Cheapest, and the credentials are dead. The repository still ships a mnemonic in its history,
which a reader may reasonably judge harshly.
Option B — rotate, remove, and rewrite history.
As above, plus
git filter-repoover the affected commits and a force-push ofmain. Thiscontradicts the repo policy "Never force-push to
main", so it needs your explicit sign-off,and anyone with an existing clone has to re-clone. Cleanest end state.
Option C — accept as-is.
Document in the README why this specific account is deliberately public and worthless, and
carve out an explicit exception in the repo's own policy notes. No work, but it leaves a
REPO_POLICIES.mdviolation in place before a 1.0 tag.Recommendation: Option A. The credentials become worthless the moment the account is
rotated, and a force-push of
mainis a bigger cost than the residual embarrassment of a deadmnemonic in history.
Whichever you pick, the follow-on work is the same and I will take it from there:
into a gated integration target,
Tell me which option and I will open the implementation work.
c is fine and is obviously my explicit decision in this greenfield project. don’t second guess my design decisions already implemented. raise them to me but no need to file it like some careless error.
Decided by @sneak: option C. Closing.
His ruling, verbatim:
> "c is fine and is obviously my explicit decision in this greenfield project. don't second guess
> my design decisions already implemented. raise them to me but no need to file it like some
> careless error."
So the credentials in
test/integration/live-login.tsare deliberate, they stay, and no historyrewrite is happening. The rotation suggestion that reached this issue second-hand is withdrawn —
it does not apply to a choice made on purpose in a greenfield repo, and it was not his.
No follow-up work is being filed off this. Option C as written mentioned documenting the rationale
in the README; he did not ask for that, and generating a documentation task out of a ruling that
the thing is fine would repeat the mistake this issue is being closed for. If he wants a line in
the README later, that is a one-minute change whenever he says so.
What I got wrong here
Worth recording plainly, since the ruling is guidance and not just a verdict.
The substance of raising it was fine. The framing was not. I opened with
REPO_POLICIES.mdquotedas "no exceptions", called it a policy violation, put it in the
1.0.0milestone as a tag blocker,and offered a history rewrite as a serious option — for a throwaway account whose own source
comment already said it was a throwaway account for integration testing. Every one of those
signals says "someone was careless", and the evidence on the page said the opposite: it was
committed intentionally, labelled as intentional, and had survived prior review.
A filed issue is a claim that something is broken and an obligation to remediate it. That is the
wrong instrument for "I noticed this and want to check it is deliberate". The right one was a
question.
Going forward in this repo: the owner's already-implemented design decisions do not get filed as
defects. When something looks wrong but is plainly deliberate — committed on purpose, internally
consistent, already reviewed — I raise it and let him decide whether it warrants an issue at all.
Genuine defects still get filed as normal, and so do choices that are merely undocumented rather
than deliberate. The distinction is whether the evidence says someone chose it.
#16 (npm publishing) stays open and genuinely with him — that one is an unmade decision about
future work, not a second-guess of something already built.