security(github-copilot): per-pod KDF for stored Copilot token

[pjdoland]

Summary

The encrypted Copilot token at ~/.jupyter/nbi/user-data.json was keyed via PBKDF2 from NBI_GH_ACCESS_TOKEN_PASSWORD whose default value is the public literal nbi-access-token-password. Every install with the default ships the same key. An attacker who could read another tenant's user-data.json (shared NFS home, accidental backup leak, errant git commit) could decrypt the OAuth access token directly: the ciphertext is decryption-equivalent to plaintext for anyone holding the known password.

Solution

Mix a per-pod + per-user secret into the KDF password input before PBKDF2. The on-disk blob format is unchanged (16 random salt bytes + Fernet ciphertext); the change is purely in what string feeds PBKDF2HMAC.derive().

Resolution order in _machine_user_secret() (first non-empty wins, the rest still contribute to the concatenation):

1. NBI_POD_IDENTITY env var (highest priority). On a multi-tenant Kubernetes deployment, /etc/machine-id typically resolves to the host node's value (shared across co-tenants) and every container runs as uid 1000. The automatic sources collapse to a value shared across co-tenants on the same node. Admins pin NBI_POD_IDENTITY to a value that stays stable across pod restarts (JupyterHub username, per-user spawn secret) for real cross-tenant isolation. Avoid metadata.uid from the downward API: that value rotates on every restart and would invalidate stored tokens on each restart.
2. /etc/machine-id or /var/lib/dbus/machine-id when mounted.
3. socket.gethostname() as a last resort. In Kubernetes the hostname equals the pod name, which differs across pods and gives per-pod isolation even though it is not a confidential value.

POSIX uid is always included. The Windows path is getattr-guarded so a missing os.getuid does not crash the read path.

New encrypt_user_secret / decrypt_user_secret helpers in notebook_intelligence/util.py wrap the existing encrypt_with_password / decrypt_with_password with the derived password. The github_copilot.py read/write call sites switch to the new helpers; tests on the bare helpers continue to cover the underlying crypto.

Legacy migration: blobs encrypted before this change decrypt via a fallback to the bare password and are silently rewritten under the v2 KDF on the next read. The legacy-fallback exception handler catches InvalidToken only, so a corrupted blob or programmer error surfaces loudly instead of being masked. The rewrite goes through the existing _save_user_data helper (introduced in PR #293) so the 0o600 file mode contract is preserved across the upgrade and a crash mid-rewrite (a K8s OOMKill during the silent upgrade) cannot leave a torn user-data.json on disk. If the rewrite itself fails (read-only filesystem, transient I/O), the read 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.

Testing

• tests/test_user_secret_kdf.py: 13 cases covering round-trip, cross-pod isolation by machine-id and uid, hostname fallback when /etc/machine-id is absent, legacy-blob fallback (and refuses with allow_legacy=False), NBI_POD_IDENTITY overriding the automatic sources, an end-to-end silent-upgrade test that drives read_stored_github_access_token against a fake user_data_file on tmp_path and verifies the on-disk bytes change between calls, and a rewrite-failure test that asserts the read still returns plaintext when write_github_access_token raises.
• Full suites: pytest 786 pass, tsc clean, prettier clean, jest clean.

Risks / follow-ups

• Behavior change: legacy blobs decrypt via fallback and are upgraded transparently; users do not have to re-login. A pod that loses its machine-id source between sessions (e.g., pod respawn without /etc/machine-id mounted, hostname changes, NBI_POD_IDENTITY unset or pointed at a rotating value like metadata.uid) will fail to decrypt v2 blobs and the user is silently logged out. The admin-guide and PRIVACY notes now document this, recommend stable NBI_POD_IDENTITY values, and explicitly warn against metadata.uid from the downward API.
• Argon2id over PBKDF2: 1.2M PBKDF2-HMAC-SHA256 iterations meet OWASP 2023 guidance. Argon2id would resist GPU/ASIC attack better; tracked as a separate hardening follow-up.
• File mode 0o600: enforced via _save_user_data (per PR security: enforce 0o600 on the encrypted GitHub token file #293) on every write including the silent legacy-blob upgrade. The merge into this branch adopts that helper at both write sites.
• Cross-tenant isolation on K8s: this PR is necessary but not sufficient when /etc/machine-id is shared across co-tenants and uid pins to 1000. The NBI_POD_IDENTITY env knob lets admins close that gap with a one-line spawn config; the admin guide section calls this out.

[mbektas]

@pjdoland this looks great but can we allow to turn off this new logic and use the existing method. In some environments filesystem is not shared and reboots change the machine id which would require a re-login.

[pjdoland]

Definitely. That is almost certainly the correct approach...as it might be for some of these other security PRS.

[pjdoland]

Converting this to a draft for now to address @mbektas's feedback before it goes in.

The ask was for a way to turn the new logic off and fall back to the existing method, since environments with a non-shared filesystem, or a machine-id that changes on reboot, would otherwise force a re-login. This branch adds NBI_POD_IDENTITY and a legacy-decrypt fallback, but neither actually disables the machine-bound KDF: the read path even re-encrypts legacy blobs into the new scheme on the next read. And because the derived secret always concatenates the machine-id and hostname, pinning NBI_POD_IDENTITY does not by itself prevent the re-login when those change.

So I'd like to add an explicit opt-out (default on, so the hardening stays the default) that uses the bare-password method when disabled and makes sure the read path does not silently re-bind in that mode. I'll mark this ready again once that is in place. Thanks for the catch.