security: enforce 0o600 on the encrypted GitHub token file#293
Merged
Conversation
~/.jupyter/nbi/user-data.json holds the encrypted GitHub Copilot access token. The previous write path was open(user_data_file, 'w') under the default umask, which on most systems produces 0o644 and leaves the ciphertext group/world-readable. On a shared-home topology (NFS, classroom labs) any other user can read the file and brute-force the default password. write_github_access_token and delete_stored_github_access_token now go through _atomic_write_json with mode=0o600 forced. The helper grew an optional mode kwarg that, when set, applies regardless of any existing file mode; this is the secrets-handling path. Default behavior (preserve existing mode) is unchanged for other callers. The mode is enforced on every write, not only on first-create, so a prior umask-default write or a manual chmod that widened the perms is tightened back to 0o600 on the next save. Tests cover first-write, loose-mode-tightened-on-rewrite, mode-preserved-through-delete, read-back roundtrip, and the no-regression contract for callers that don't pass mode.
Six-agent review polish: extract a single helper so the 0o600 mode contract lives in one place. A future third writer of user-data.json can't accidentally drop the mode kwarg.
mbektas
approved these changes
May 18, 2026
pjdoland
added a commit
to pjdoland/notebook-intelligence
that referenced
this pull request
May 18, 2026
Second-pass review surfaced two real issues: 1. Merge conflict against main showed this branch's write path was bypassing the 0o600 mode contract that PR plmbr#293 introduced via the `_save_user_data` helper. After the silent legacy-blob upgrade re-encrypted under the per-pod KDF, it would land the rewrite at the default umask (typically 0o644), undoing the file-mode hardening on every legacy-format read. Resolve the merge by routing both write call sites through `_save_user_data` so the mode contract holds across the upgrade. 2. The admin guide recommended `metadata.uid` from the Kubernetes downward API as an `NBI_POD_IDENTITY` value, but `metadata.uid` rotates on every pod restart. That would invalidate the stored token on every restart and silently log the user out. Recommend stable per-user values (JupyterHub username, per-user spawn secret) and call out the rotation hazard explicitly. New test pins the silent-upgrade rewrite fail-open contract: when `write_github_access_token` raises (read-only filesystem, transient I/O), `read_stored_github_access_token` still returns the decrypted plaintext so the user's login completes; the next read falls back to the legacy path again until the underlying I/O problem clears.
This was referenced May 18, 2026
pjdoland
added a commit
to pjdoland/notebook-intelligence
that referenced
this pull request
May 22, 2026
Promotes the [Unreleased] CHANGELOG snapshot to [5.0.0] - 2026-05-22 and expands it to cover everything merged into upstream/main after PR plmbr#287's docs refresh. Bumps package.json to 5.0.0. CHANGELOG additions cover the post-plmbr#287 surface: - Settings tabs: plugin marketplace picker (plmbr#284), plugin marketplace details + Update button (plmbr#303), per-workspace MCP disable (plmbr#286), JSON-paste path in Add MCP server (plmbr#285). - Launchers: hide-with-policy (plmbr#288), brand icons for Codex / opencode (plmbr#325, plmbr#333), per-launch directory picker (plmbr#332). - Chat sidebar and agentic UX: workspace @-mention in Claude mode (plmbr#327), reload-open-files-on-disk (plmbr#330), steered system prompt away from over-eager notebook creation (plmbr#336). - Skills: multi-manifest support (plmbr#321), tracks-upstream for user- imported skills (plmbr#322), HTTP kill switch for the reconciler (plmbr#291). - Accessibility: full sub-section covering plmbr#305-plmbr#320. - Security: shell-tool sandbox (plmbr#290), Claude UI-bridge sandbox (plmbr#323), 0o600 on encrypted token (plmbr#293), env-secret scrubbing (plmbr#295), MCP config shape validation (plmbr#299), XSS allowlist (plmbr#296), Copilot WS auth + origin (plmbr#301), GHE host detection (plmbr#292), fastmcp -> mcp SDK swap (plmbr#324). - Fixed: session listing unification (plmbr#310), session preview unwrap (plmbr#331), down-area runtime throw (plmbr#330 follow-up), WS message-handler leak (plmbr#294). - Removed: fastmcp dependency, history.jsonl session gate. Adds a Migration note covering the five behavior changes operators should review before upgrading from 4.x: fastmcp swap, path sandboxes, history.jsonl gate removal, workspace @-mention pointer shape, and the Copilot WebSocket auth/origin tightening. Two reviewer rounds (six personas each) applied: - Round 1 caught security overclaims (plmbr#293, plmbr#299, plmbr#323), the plmbr#284/plmbr#303 mis-attribution, missing migration note, 3 em dashes, and the stale `fastmcp==2.x.*` recommendation in the admin guide. - Round 2 caught the missing plmbr#301 migration bullet, missing version- matrix 5.0.x row, missing README TOC entry, and a couple of style nits (sub-heading overpromise, orphan bullet). Skipped (deferred to future PRs): - README first-run tour mention. - Admin guide HTTP kill-switch row in Failure-modes table. - Terminal drag-drop trust-model precision update after plmbr#327. - Cipher description nit in plmbr#293 (Fernet AES-128-CBC+HMAC, not AES-GCM).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`~/.jupyter/nbi/user-data.json` holds the encrypted GitHub Copilot access token. The previous write path used `open(user_data_file, 'w')` under the default umask, which on most systems produces `0o644` and leaves the ciphertext group/world-readable. On a shared-home topology (NFS, classroom labs) any other user can read the file and brute-force the default password.
Solution
Both write sites (`write_github_access_token` and `delete_stored_github_access_token`) now go through `_atomic_write_json` with `mode=0o600` forced. The helper grew an optional `mode` kwarg that, when set, applies regardless of any existing file mode. Default behavior (preserve existing mode) is unchanged for other callers, so the config-file path stays backward-compatible.
The mode is enforced on every write, not only on first-create. A prior umask-default write or a manual `chmod` that widened the perms is tightened back to `0o600` on the next save.
Testing
Four new pytest cases in `tests/test_github_copilot_user_data_mode.py` (first-write, loose-mode-tightened-on-rewrite, mode-preserved-through-delete, read-back roundtrip) plus three new cases in `tests/test_config_atomic_save.py` covering the new `mode` kwarg (override, first-create, no-mode-arg backward compat). Confirmed regression detection by reverting `github_copilot.py` to upstream/main and re-running: 3 of 4 user-data tests fail.
`pytest tests/` (739 passing, +7 new), `jlpm tsc --noEmit`, `jlpm lint:check`, `jlpm jest` (154 passing). All four green.
Risks / follow-ups