fix(cli): credentials to flat ~/.config/dimos-credentials, plain text - #3549
Conversation
The login PR nested credentials.json under ~/.config/dimos, turning the dimos-run config FILE path into a directory on any logged-in keyring-less machine (dimos run then dies with IsADirectoryError; reverse direction breaks login on machines with a legacy config file). Credentials are now a plain-text key in a flat 0600 sibling — no contested path, no JSON, no migration shims. Machines that logged in during the collision window just run dimos login again.
Greptile SummaryThe credential-format migration prevents users with an existing JSON-formatted cloud login from authenticating. The loader must preserve compatibility by extracting the stored Confidence Score: 4/5Not safe to merge until existing JSON-formatted credentials continue to authenticate. One confirmed, non-security authentication compatibility failure remains: legacy credential objects are sent verbatim instead of using their API key. Files Needing Attention: dimos/cli/cloud.py
What T-Rex did
|
| if key := kr.get_password(_KEYRING_SERVICE, _KEYRING_USER): | ||
| return cast("str", key) | ||
| try: | ||
| return cast("dict[str, str]", json.loads(CREDENTIALS_PATH.read_text())) | ||
| except (OSError, ValueError): | ||
| return CREDENTIALS_PATH.read_text().strip() or None |
There was a problem hiding this comment.
Legacy JSON logins send a JSON document as the bearer token
Users authenticated by the preceding implementation have {"api_key":"...","email":"..."} stored in the legacy file or keyring. _load() now returns that whole object as a string, so api_key() passes it through and whoami() sends Bearer {"api_key":...} instead of the API key. Existing logged-in users will therefore be rejected until they manually log in again. Decode the legacy JSON shape when it contains a string api_key (optionally rewriting it in the new format), while preserving new plain-text credentials.
Artifacts
Isolated legacy JSON credential reproduction script
- The authored no-network harness loads both parent and current cloud modules, exercises file and keyring legacy values, and records the Authorization header, with the takeaway that the regression is directly executable.
Before-change legacy JSON credential behavior
- The parent-revision run shows both legacy file and keyring credentials are decoded to `dimos_sk_old` and sent as `Bearer dimos_sk_old`, with the takeaway that the prior behavior authenticated correctly.
Current-change legacy JSON credential behavior
- The current-revision run shows both legacy file and keyring values are returned and sent as the full JSON blob, with the takeaway that existing users' cloud authentication regresses.
Codecov Report✅ All modified and coverable lines are covered by tests. @@ Coverage Diff @@
## main #3549 +/- ##
==========================================
- Coverage 76.13% 76.12% -0.01%
==========================================
Files 1228 1228
Lines 119174 119172 -2
Branches 10684 10684
==========================================
- Hits 90731 90722 -9
- Misses 25342 25349 +7
Partials 3101 3101
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes 🚀 New features to boost your workflow:
|
| try: | ||
| return cast("dict[str, str]", json.loads(CREDENTIALS_PATH.read_text())) | ||
| except (OSError, ValueError): | ||
| return CREDENTIALS_PATH.read_text().strip() or None |
There was a problem hiding this comment.
A focused local HTTP-server check exercised load → apikey → whoami with both simulated...
- Bug
- A focused local HTTP-server check exercised load → apikey → whoami with both simulated keyring storage and the fallback credential file. A plain token produced Authorization: Bearer current-token, while the legacy value {"apikey":"legacy-secret","email":"legacy@example.test"} produced Authorization: Bearer {"apikey":"legacy-secret","email":"legacy@example.test"} in both storage modes. This confirms that existing JSON-formatted logins cannot authenticate because the complete document, rather than its apikey, is used as the bearer token.
- Cause
- T-Rex reproduced this while running the changed behavior, but it did not return a separate root-cause sentence.
- Fix
- Update the changed code so this failing path is handled, then rerun the same T-Rex check to confirm it passes.
Artifacts
Focused legacy JSON credential end-to-end test source
- This exact temporary test source loads the repository's cloud module, simulates keyring and file credentials, and captures the local server Authorization header; the takeaway is that it exercises the reported path without changing repository code.
Baseline scalar credential execution output
- This executed baseline run records `_load`, `api_key`, and the captured Authorization header for a normal scalar token in both storage modes; the takeaway is that the expected `Bearer current-token` path works.
Legacy JSON credential execution output
- This executed run records `_load`, `api_key`, and the local server's captured Authorization header for legacy JSON in keyring and file storage; the takeaway is that both send the whole JSON object as the bearer token.
didn't realize dimos in .config was a file, assumed we had a config dir