Repository navigation
bug(slm): importing services.* generates and writes secret keys, so a read-only import mutates the tree #17482
Description
Defect
import services.<anything> executes services/__init__.py, which reaches code that auto-generates SLM secret keys and persists them:
SLM secret keys were not set via environment variables. Keys have been
auto-generated and saved to <backend>/data/.slm_keys
No key material was requested. The import was for an unrelated leaf helper.
How it cost a deployment today
A read-only verification — importing services.git_subprocess against the deployed tree to confirm #17457's sys.path fix — created a 600 file owned by the diagnostic user, in a directory where everything else is autobot:autobot.
The code-sync rsync runs as autobot, could not read it, and the pull failed:
rsync: [sender] send_files failed to open ".../data/.slm_keys": Permission denied (13)
Pull failed
An import with no side effects in its name broke the update path for the whole node.
Why this is more than an ownership accident
The same write happens to anyone who imports that package from a shell, a script, or a test run outside the service's own environment — with whatever uid they happen to have. The failure surfaces later, somewhere unrelated, as a permissions error on a file nobody remembers creating.
Generating a secret is a decision. Persisting one is a bigger decision. Doing both as a side effect of resolving an import means the decision is made by whoever happens to touch the module, in whatever directory they happen to be in.
Acceptance criteria
- Importing
services.*performs no key generation and no disk write. Generation moves behind an explicit call the service makes at startup. - If auto-generation stays for developer convenience, it is opt-in via a flag or env var and never the default on import.
- The write, wherever it ends up, refuses when the target directory's ownership does not match the service user — rather than creating a file the service cannot read.
- A test that imports the package in a temp cwd and asserts no file was created. That arm has no coverage today, and it is what would have caught this.
- Sweep for other import-time writes in the SLM backend; state the count so none found is distinguishable from did not look.
Stated limits
The stray file still exists. Clearing it needs elevated permission and is a credentials deletion, which an agent proposes rather than performs (#17038). The real keys at /etc/autobot/slm-secrets.env are unaffected, and the path is gitignored at .gitignore:133, so nothing was ever repository content.
Activity
mrveiss commentedon Sep 25, 2026
Second independent occurrence, different session, same day. This is no longer a one-off.
A second session importing a module from the codebase — to check an unrelated fact about rsync excludes — triggered the same auto-generation and write. Contained in a worktree this time, gitignored and untracked, but the mechanism is identical: an import with no side effects in its name created key material on disk as whatever user happened to run it.
Two occurrences in a few hours, by two sessions, neither of them asking for a key. The first cost a failed
code_sourcersync and blocked deployment; the second cost nothing only because of where it happened to run.That changes the severity argument. One instance reads as carelessness by the person who ran the import. Two, independently, reads as what it is: the module cannot be imported without writing secrets, so every importer is exposed and the outcome depends only on their cwd and uid. Test runners, scripts, and tooling all import; the failure surfaces later as a permissions error on a file nobody remembers creating.
Worth adding to the acceptance criteria, since the second session did the right thing and it still happened: the fix cannot be "do not import it". Both sessions were doing read-only diagnostics, both knew the codebase rules, and neither had any signal that an import was a write. A rule cannot defend against a side effect that is invisible at the call site.
The second session also re-derived its conclusion statically rather than resting on the import, and reported the write to its owner for disposal rather than deleting credentials itself — correct under #17038, and the reason nothing further was lost.