Predicate for the refresh-daemon work (deliverable 0 in refresh_daemon_plan.md):
XDG config-path support and non-clobbering key-file naming. Rev 2 folds in the
adversarial review (verdict SOUND-WITH-FIXES) and the owner's I7 ruling.
- Existing
~/.opk/configurations continue to work untouched. Any behavior change anywhere must be explicitly identified, never silent. - Multiple identities coexist on disk (daemon multi-session prerequisite).
- Single-identity users keep byte-identical behavior until a second identity actually appears.
- opkssh never writes to
~/.ssh/configoutside the explicit--configureopt-in. - XDG base-directory conventions adopted — strictly: this PR corrects the project's non-conformance, it does not invent a variant.
--config-path flag, else first existing of:
<configDir>/opk/config.ymlwhere<configDir>=$XDG_CONFIG_HOMEwhen set and absolute (the XDG spec requires relative values to be ignored), else the platform default:~/.configon Unix-likes (macOS included — CLI convention),%AppData%on Windows. Replacement semantics, not stacking: a set variable replaces~/.config, matching the spec and git/gh.- Legacy
~/.opk/config.yml(all platforms — this is the code truth today; the current--config-pathhelp text claiming%APPDATA%\.opkon Windows is wrong upstream and gets corrected here).
When none exists, resolution falls to candidate 1, where --create-config
then writes (dir 0700, file 0600 — the config can carry client_secret; the
current 0755/0644 is tightened for new creates). A present legacy file always
wins and is never moved. If the home directory cannot be determined, the
legacy candidate is dropped loudly (logged), never silently. At verbosity
≥1, login logs using client config at <path> — this defuses the
silent-shadowing class (e.g. a user following the provider guides hand-creates
the legacy file while a config already exists at candidate 1).
Unified rule — file names never change until a second identity forces disambiguation:
- Default tier (
~/.ssh/): default names (id_ecdsa, …) preferred, exactly as today. A slot is reusable iff free, or held by the same identity — (iss, aud, sub) parsed fail-closed from the existing cert's embedded PK token — or held by a cert whose PK token does not parse but whose comment is exactlyopenpubkey(legacy fallback: byte-identical to today's comment check for every old file, so requirement 3 never rests on cross-version PK-token wire-format stability). A slot held by a different identity or a foreign key is never overwritten. - Fallback = the opkssh identity directory (
~/.ssh/opkssh/, owner ruling on review finding I7): when every default slot belongs to someone else, login writes through the same code path the--configuretier uses —~/.ssh/opkssh/<issuer-host>-<clientid20>[-<idtag>]— creating the directory (0700) and fragment file if absent. TheIdentityFilefragment line is inert without theIncludein~/.ssh/config(requirement 4 untouched) but makes a later--configureadopt every key instantly. No files are ever written to the~/.sshroot under non-default names, which also removes any newlogoutsweep: the existing opkssh-dir removal already covers the fallback location, and no-arglogoutgains no new reach over user-named files. --configuretier: unchanged naming, gaining the same guard — a same-name file (private key or cert present) belonging to a different or unparseable identity disambiguates to…-<idtag>.<idtag>= first 8 hex of sha256(issuer|audience|subject) — the full triple, notsubalone, so the 20-char audience truncation cannot alias two client_ids, and no PII lands in file names. The tagged path is then re-checked with the same slot rule: if it exists and is not the same identity, login errors out rather than overwriting — the clobber this PR kills must not return one tier down (collision odds are astronomically low; erroring is honest, silent replacement is not).- Fragment hygiene: the
IdentityFilededup matches whole lines, not substrings (a tagged path contains its untagged prefix). - Discovery & warning UX: labeled keys are reached via the agent
(PR #611 loads it at login), an explicit
IdentityFile,-i, or--configure. The remediation warning (naming the written path and the three remedies) is decided after the agent-add attempt: it fires when the fallback was used and the key did not actually land in an agent — covering both no-agent and agent-dead, and staying silent when the agent has the key.
pktIdentity/certFileIdentity return (identity, ok) and fail closed:
extraction failures never compare equal (equality authorizes overwrites), and
a nil PK token is a non-identity, not a panic. Parse failures on existing
files keep today's diagnostics (Failed to parse: <path>).
| # | Location | Change |
|---|---|---|
| 1 | commands/config/client_config.go |
two-candidate chain, absolute-only XDG, loud legacy-drop, fs-aware ResolveClientConfigPath(afero.Fs, *string), create perms 0700/0600 |
| 2 | commands/config/config_path_unix.go / _windows.go (new) |
platform userConfigDir |
| 3 | commands/login.go:174 |
resolution caller passes l.Fs; verbosity≥1 using client config at log |
| 4 | commands/login.go writeKeysToSSHDir |
same-identity slot rule + openpubkey-comment legacy fallback; overflow delegates to the opkssh-dir writer (no root-dir labeled files) |
| 5 | commands/login.go writeKeysToOpkSSHDir |
create-if-absent dir (0700) + fragment; different/unparseable-identity guard (key or cert present); idtag disambiguation + tagged-path re-check (error on foreign occupant); whole-line fragment dedup |
| 6 | commands/login.go helpers |
keyIdentity, fail-closed pktIdentity/certFileIdentity, `identityTag(iss |
| 7 | commands/login.go addCertToAgent + login() |
agent-add reports success; fallback warning decided after the attempt |
| 8 | commands/logout.go |
no change (rev 2: the draft's root-dir sweep is reverted; existing opkssh-dir removal covers the fallback) |
| 9 | main.go:212,485 |
--config-path help → the chain (fixes existing Windows inaccuracy); flag exists on login and client provider list |
| 10 | commands/login.go:~1090 |
scope-troubleshooting message: resolved-path wording |
| 11 | docs/config.md, README.md:445 |
resolution chain documented (README's Windows claim also currently wrong) |
| 12 | docs/providers/azure.md:87,144, cognito.md:125, keycloak.md:61 |
~/.opk/config.yml instructions → chain-aware wording (rev 2, from review I6) |
| 13 | docs/cli/*.md |
regenerate via opkssh gen-docs after help-text change |
| 14 | README.md, docs/cli/opkssh_{login,logout,inspect}.md, docs/opkssh-and-sssd.md, docs/putty.md |
id_ecdsa references reviewed; add multi-identity note where login flow is described |
| 15 | tests | see below |
Explicit non-touchpoints: ~/.opk/auth_id + readhome/audit/policy
(server-side contract); /etc/opk/*; the --configure Include mechanism;
the -i tier; PR #611's agent add semantics; all three existing logout
removal flows.
- Config chain (memfs +
t.Setenv): XDG set-absolute / set-relative (ignored) / unset × file-presence matrix; replacement semantics (XDG set +~/.configfile present →~/.configNOT consulted); legacy-wins-on-read; none-exist resolves to candidate 1; explicit--config-pathuntouched. - Key writing: same-identity re-login overwrites
id_ecdsain place; legacy-comment cert with unparseable PKT still overwritten in place (requirement-3 regression guard); different identity → dir-placed fallback,id_ecdsapreserved, fragment line added, dir/fragment created when absent; idtag determinism + no raw sub; tagged-path foreign occupant → error; opkssh-dir guard with cert-missing/key-present partial state; fragment whole-line dedup. - Warning: fires on fallback + agent-add failure; silent on fallback + agent-add success and on default-slot writes.
- Logout: regression only — existing flows untouched, fallback files removed by the existing opkssh-dir sweep.
Config migration tooling (legacy → XDG copy), daemon sock_path/state dirs
(daemon PR), Windows key-path changes beyond the config dir, any
~/.ssh/config writes.