feat(login): send device_label so users can identify CLI sessions

[rsnodgrass]

Fixes #625 (CLI half).

/settings/security lists every PAT minted by ox login with nothing but timestamps. Someone with a laptop, a desktop, a devcontainer and a CI runner cannot tell which row to revoke. This sends a short user@host label with the device-flow token exchange.

One edit covers both login entry points — the package-level Login() and (*AuthClient).Login() share pollToken.

⚠️ Needs your sign-off before merge

• New customer-facing env var: SAGEOX_NO_DEVICE_LABEL. Per CLAUDE.md, any new SAGEOX_* var needs your review. I took the reversible defaults rather than blocking: opt-out (not opt-in), default label user@host degrading to host-only.
• Backend does not accept the field yet (separate sageox-mono issue, per the issue's own scope split). Standard JSON decoders ignore unknown fields and the CLI reads nothing new off the response, so this is purely additive. The one real risk: a backend using strict decoding (DisallowUnknownFields / Zod .strict()) would 400 and break ox login outright. Worth a yes from the mono owner before merge.

Why user@host and not hostname-only

Hostname alone answers the question for a laptop fleet and fails exactly the cases that need it most — shared boxes, CI runners, devcontainers, and multi-account workstations. It's also the format users already read in GitHub SSH key labels, 1Password and ssh itself. Degrades gracefully: host-only → user-only → empty.

Where the username comes from — and three sources deliberately rejected

The OS account name ($USER, then $USERNAME). Explicitly not:

Rejected	Why
signature.GetMachineID()	It is the HMAC key for the local signature cache — transmitting it leaks a local secret. Also embeds a random UUID that means nothing in a UI.
identity.AttributionName	Documented LOCAL ONLY in that file's own privacy taxonomy.
identity.AttributionUsername	Resolves through git config, so the label would depend on which directory you happened to run ox login from — non-deterministic, and more identifying than the account name.

Privacy — meeting ADR-008 head-on

A reviewer will reach for "we never collect machine identifiers". That table sits under Telemetry, and the distinction is the scope of that rule, not an exception to it:

	Telemetry	Device label
Audience	SageOx, aggregated across users	The user, on their own account page
Identity	Deliberately anonymous	Already authenticated — the server knows who
Risk of a machine id	Correlates an anonymous user's sessions	None; identity is already established

A stable machine id is banned in telemetry precisely because it de-anonymizes an otherwise anonymous stream. That hazard doesn't exist for metadata attached to a credential the user is knowingly creating. The label answers which machine, never who.

This PR ships the ADR amendment (new §6 + a summary-matrix row) rather than leaving the scoping to be inferred from section nesting.

Data minimization applied: first DNS label only (laptop, not laptop.corp.internal — the domain identifies an employer more than a machine); allowlisted to [A-Za-z0-9._-]; truncated to 32 (user) + 63 (host, the DNS label max, so no legal hostname is ever cut).

Sanitization is an allowlist, not a denylist

A hostname is attacker-influenceable on a shared box, and the label is transmitted then rendered in a web UI. The allowed set is the intersection of what's safe in a GitLab PAT name, an HTTP header, a JSON string and a URL segment — so everything below 0x20, 0x7F, and all non-ASCII is excluded by construction:

Input	Output
a\r\nX-Evil: 1	a-X-Evil-1
"><script>alert(1)</script>	script-alert-1-script
ryan@corp	ryan-corp — an @ in a part is neutralized, so the single @ in the output is always the composer's
200-char hostname	truncated to 63

Test Plan

• TestPollToken_SendsDeviceLabel — decodes the actual request body; asserts the pre-existing fields are untouched and the label matches the shape contract.
• TestPollToken_OmitsDeviceLabelWhenOptedOut — asserts the key is absent, not present-and-empty. That's what proves omitempty: an empty string would still be a wire change, which an opt-out must not be.
• TestDeviceLabel_HostileHostnameStillProducesASafeLabel — property test over injection payloads.
• TestOptedOutOfDeviceLabel — pins truthiness, including that SAGEOX_NO_DEVICE_LABEL=0 means labels on, as it reads.
• internal/auth/env_naming_test.go (the OX_* guard) still passes; make lint clean.

Follow-up, deliberately out of scope

Not sending the label on the earlier /api/auth/device/code request. Showing "you are authorizing ryan@laptop" on the approval screen before consent is genuinely better security UX, but it needs a second backend change and an approval-screen design decision.

Summary by CodeRabbit

• New Features

  • Login requests now include a sanitized device label when available, helping identify the associated device.
  • Labels follow safety and length limits and are stored separately from telemetry.

• Privacy

  • Set SAGEOX_NO_DEVICE_LABEL=1 to prevent device labels from being sent during login.
  • Documented device-label handling, transmitted data, storage, and opt-out behavior, including CI/CD guidance.

• Tests

  • Added coverage for label formatting, safety limits, hostile input, and opt-out behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 6233f9a6-0d06-4947-999c-4ce93218ab65

📥 Commits

Reviewing files that changed from the base of the PR and between 2a94cea and 918455f.

📒 Files selected for processing (7)

• docs/adr/008-privacy.md
• docs/debugging.md
• docs/guides/ci-pipelines.md
• internal/auth/device_flow.go
• internal/auth/device_flow_label_test.go
• internal/auth/device_label.go
• internal/auth/device_label_test.go

🚧 Files skipped from review as they are similar to previous changes (7)

• docs/guides/ci-pipelines.md
• internal/auth/device_label.go
• docs/debugging.md
• internal/auth/device_flow_label_test.go
• docs/adr/008-privacy.md
• internal/auth/device_flow.go
• internal/auth/device_label_test.go

---

📝 Walkthrough

Walkthrough

ox login now derives a sanitized, optionally disabled user@host device label and includes it in token exchange requests. Tests cover payload behavior and safety constraints. Documentation describes transmission, minimization, and opt-out behavior.

Changes

Device label login flow

Layer / File(s)	Summary
Device label generation and validation internal/auth/device_label.go, internal/auth/device_label_test.go	Device labels use sanitized username and hostname components, support environment-based opt-out, and enforce safe character and length limits.
Token exchange payload and coverage internal/auth/device_flow.go, internal/auth/device_flow_label_test.go	Token polling includes an optional device_label field and verifies normal transmission and omission when opted out.
Privacy and environment documentation docs/adr/008-privacy.md, docs/debugging.md, docs/guides/ci-pipelines.md	Documentation describes device-label privacy handling, minimization rules, and SAGEOX_NO_DEVICE_LABEL.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ox_login
  participant deviceLabel
  participant pollToken
  participant TokenEndpoint
  ox_login->>pollToken: start token polling
  pollToken->>deviceLabel: generate optional label
  deviceLabel-->>pollToken: sanitized label or empty value
  pollToken->>TokenEndpoint: submit token request
  TokenEndpoint-->>pollToken: return token response

Loading

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 44.44% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: sending device labels during login for CLI session identification.
Linked Issues check	✅ Passed	The PR implements the CLI scope in issue #625 by sending sanitized device_label values during token exchange with opt-out support.
Out of Scope Changes check	✅ Passed	The documentation, implementation, and tests support issue #625 and do not introduce backend, web UI, or unrelated changes.

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ryan/gh-625-device-label

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

internal/auth/device_label_test.go (1)

68-80: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assertions are skipped whenever the resolved part is empty — risk of a vacuous pass.

In TestDeviceLabel_FallsBackWhenUsernameUnavailable (68-80) and TestDeviceLabel_HostileHostnameStillProducesASafeLabel (122-159), the meaningful checks are gated behind if got != "" / if part == "" { continue }. If os.Hostname() legitimately resolves empty in a sandbox, or a hostile input sanitizes fully to "", the test silently verifies nothing for that case and still passes. Also, the USERNAME (Windows) fallback branch in device_label.go (lines 74-76) has no dedicated test — only USER is exercised directly.

Consider making hostname/user resolution overridable (e.g. package-level var lookupHostname = os.Hostname) so both the "host present" and "host absent" paths can be asserted deterministically instead of relying on ambient environment, and add a case that sets USER=""/USERNAME=<value> to cover the Windows fallback.

Also applies to: 122-159

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/auth/device_label_test.go` around lines 68 - 80, Update deviceLabel
test coverage to avoid vacuous assertions: make hostname lookup injectable via a
package-level lookupHostname variable used by deviceLabel, then stub it to test
both non-empty and empty hostname outcomes explicitly. Remove conditional
assertion skips in TestDeviceLabel_FallsBackWhenUsernameUnavailable and
TestDeviceLabel_HostileHostnameStillProducesASafeLabel, asserting the required
result contract for every case, and add a dedicated USER empty/USERNAME
populated case covering the Windows fallback.

Source: Learnings

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/adr/008-privacy.md`:
- Around line 358-369: Update the privacy comparison and surrounding explanation
to acknowledge that the credential label may include USER/USERNAME and therefore
potentially identify the local OS account. Replace the “None” machine-identifier
risk and “never who” claims with wording that accurately distinguishes machine
identification from possible account-name disclosure, while preserving the
distinction from anonymous telemetry.

---

Nitpick comments:
In `@internal/auth/device_label_test.go`:
- Around line 68-80: Update deviceLabel test coverage to avoid vacuous
assertions: make hostname lookup injectable via a package-level lookupHostname
variable used by deviceLabel, then stub it to test both non-empty and empty
hostname outcomes explicitly. Remove conditional assertion skips in
TestDeviceLabel_FallsBackWhenUsernameUnavailable and
TestDeviceLabel_HostileHostnameStillProducesASafeLabel, asserting the required
result contract for every case, and add a dedicated USER empty/USERNAME
populated case covering the Windows fallback.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: fe532751-8bae-435b-8205-3a10fbd45356

📥 Commits

Reviewing files that changed from the base of the PR and between 4c451dd and 2a94cea.

📒 Files selected for processing (7)

• docs/adr/008-privacy.md
• docs/debugging.md
• docs/guides/ci-pipelines.md
• internal/auth/device_flow.go
• internal/auth/device_flow_label_test.go
• internal/auth/device_label.go
• internal/auth/device_label_test.go

[rsnodgrass]

Fixed — you were right, and it mattered.

user@host includes the OS account name, so "never who" and "risk: None" were both overstatements in a privacy doc, which is the worst place to overstate.

ADR-008 §6 now says plainly that the label identifies the machine and the account on it — for a single-user laptop, a person — and that it is not anonymous. The argument is narrowed to the thing that's actually true: the device-flow exchange is already authenticated, so the label adds which device without adding linkability the server didn't already have. The residual disclosure (the local account name itself) is named explicitly as the reason the opt-out and the first-DNS-label reduction exist. Same correction applied to the deviceLabel() doc comment and the summary-matrix row.

[rsnodgrass]

Backend check: safe to merge, but inert until sageox-mono changes

Verified against sageox-mono (apps/web/app/api/v1/device/token/route.ts):

No strict decoding — zero break risk. The handler does const body = await request.json() and destructures { device_code }. No zod, no DisallowUnknownFields, no schema validation. An unknown device_label is silently ignored. The risk I flagged in the original description does not exist.

But the field goes nowhere. The route doesn't forward the client's body — it constructs its own internal request to Better Auth with an explicit {device_code, grant_type, client_id}. So device_label is dropped at the first hop.

So this PR is groundwork, not a working feature. It's safe to land, but nothing changes in /settings/security until mono is updated to accept the field, persist it on gitlab_pats.name, return it in cliSessionResponse, and render it. (PAT naming is still timestamp-only at apps/api-go/internal/service/gitlab.go:474, so the issue's premise does hold.) Tracked as ox-11pk.

Second thought on SAGEOX_NO_DEVICE_LABEL — the shape is wrong

I shipped the env var because #625 specified it, but ox's established precedent for a privacy toggle is command + user-config + env var — telemetry has ox telemetry off, UserConfig.TelemetryEnabled, and SAGEOX_TELEMETRY. An env var alone is the weakest third: undiscoverable, not persistent across shells, invisible in ox config, easy to forget in CI.

Worth weighing against that: ox already sends far more identifying data to this same authenticated backend — git remote URLs (org names), repo names, session transcripts containing file paths and code. A short hostname plus OS account name isn't incrementally identifying against that baseline, and nothing else ox sends has a per-field opt-out.

Two coherent options; the current half-precedent is the one that isn't:

1. Drop the env var — no opt-out, consistent with every other field ox transmits.
2. Promote to UserConfig, env var stays as the CI override — consistent with telemetry.

Since the label is inert today there's no urgency either way. Flagged as ox-kttu for your call; happy to implement whichever.