Skip to content

fix(cli): credentials to flat ~/.config/dimos-credentials, plain text - #3549

Merged
spomichter merged 2 commits into
mainfrom
fix/credentials-flat-file
Aug 19, 2026
Merged

fix(cli): credentials to flat ~/.config/dimos-credentials, plain text#3549
spomichter merged 2 commits into
mainfrom
fix/credentials-flat-file

Conversation

@spomichter

@spomichter spomichter commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

didn't realize dimos in .config was a file, assumed we had a config dir

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-apps

greptile-apps Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

The credential-format migration prevents users with an existing JSON-formatted cloud login from authenticating. The loader must preserve compatibility by extracting the stored api_key before making cloud requests.

Confidence Score: 4/5

Not 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

T-Rex T-Rex Logs

What T-Rex did

  • T-Rex produced a proof for a posted P1 finding, including the focused legacy JSON credential end-to-end test source and its execution outputs.
  • T-Rex produced a second proof for a posted P1 finding.
  • The general-contract-validation-proof shows that the CLI passes the entire legacy credential JSON as the token and confirms no repository changes beyond creating proof artifacts.

View all artifacts

T-Rex Ran code and verified through T-Rex

Comments Outside Diff (1)

  1. General comment

    P1 Legacy JSON credentials are sent as literal bearer tokens

    • Bug
      • A stored legacy credential object such as {"api_key":"legacy-secret","email":"legacy@example.test"} reaches the outgoing /auth/whoami request as Authorization: Bearer {"api_key":"legacy-secret","email":"legacy@example.test"}. This occurs for both keyring and file fallback storage, so previously stored legacy logins cannot authenticate.
    • Cause
      • _load() returns the raw stored string without recognizing/parsing JSON, api_key() returns that value unchanged, and whoami() interpolates it directly into the Authorization header.
    • Fix
      • In _load(), detect and parse legacy JSON-object credential values from both keyring and file storage; validate and return the non-empty string api_key field, while preserving current plain-token behavior.

    T-Rex Ran code and verified through T-Rex

Reviews (2): Last reviewed commit: "drop editorializing comments" | Re-trigger Greptile

Comment thread dimos/cli/cloud.py
Comment on lines +82 to +85
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 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.

View artifacts

T-Rex Ran code and verified through T-Rex

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

@@            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              
Flag Coverage Δ
OS-ubuntu-24.04-arm 70.54% <100.00%> (-0.01%) ⬇️
OS-ubuntu-latest 72.32% <100.00%> (-0.01%) ⬇️
Py-3.10 72.32% <100.00%> (+<0.01%) ⬆️
Py-3.11 72.31% <100.00%> (-0.01%) ⬇️
Py-3.12 72.32% <100.00%> (-0.01%) ⬇️
Py-3.13 72.31% <100.00%> (-0.01%) ⬇️
Py-3.14 72.32% <100.00%> (-0.01%) ⬇️
Py-3.14t 72.32% <100.00%> (-0.01%) ⬇️
SelfHosted-Large 29.74% <17.64%> (+<0.01%) ⬆️
SelfHosted-Linux 35.77% <17.64%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
dimos/cli/cloud.py 84.78% <100.00%> (-0.33%) ⬇️
dimos/cli/test_cloud.py 100.00% <100.00%> (ø)
dimos/constants.py 76.00% <100.00%> (ø)

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread dimos/cli/cloud.py
try:
return cast("dict[str, str]", json.loads(CREDENTIALS_PATH.read_text()))
except (OSError, ValueError):
return CREDENTIALS_PATH.read_text().strip() or None

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P1 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.

View artifacts

T-Rex Ran code and verified through T-Rex

@spomichter
spomichter merged commit 3d7869f into main Aug 19, 2026
32 of 33 checks passed
@spomichter
spomichter deleted the fix/credentials-flat-file branch August 19, 2026 13:18
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