feat(auth): OAuth profiles with --profile flag and auth subcommands

[axisrow]

Summary

• Add direct auth login/list/use/status with OAuth PKCE, custom-app and ready-token flows.
• Introduce per-profile credential resolver (OAuth store + YANDEX_DIRECT_TOKEN_<SUFFIX> env) and top-level --profile flag.
• Persist profiles to ~/.direct-cli/auth.json with 0600/0700 permissions; active profile triggers early credential resolution.
• Document the flow in README (EN + RU) and cover with 13 new unit tests (57/57 pass, flake8/black clean).

Test plan

• pytest tests/test_auth_oauth.py tests/test_cli.py tests/test_comprehensive.py -v — 57 passed
• flake8 direct_cli tests — clean
• black --check direct_cli tests — clean
• Manual: direct auth login --oauth-token y0_… --profile sandbox → direct auth list → direct --profile sandbox campaigns get

🤖 Generated with Claude Code

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 3m 9s —— View job

---

PR Review: feat(auth): OAuth profiles with --profile flag and auth subcommands

Tasks:

• Gather context and understand the request
• Read changed files: direct_cli/auth.py
• Read changed files: direct_cli/commands/auth.py
• Read changed files: direct_cli/cli.py, direct_cli/api.py
• Read test files
• Analyze for critical issues
• Post review findings

---

Critical Issues

1. Token file permissions race condition (Security — auth.py:139-143)

def _write_json(path: Path, payload: Dict[str, Any]) -> None:
    path.parent.mkdir(parents=True, exist_ok=True)
    os.chmod(path.parent, 0o700)
    path.write_text(...)          # file created with umask permissions (often 0644)
    os.chmod(path, 0o600)         # ← too late; file was already briefly world-readable

path.write_text() creates the file with default umask-derived permissions (typically 0644 — readable by all users). The os.chmod(0o600) call comes after, leaving a race window where another local process can read the plaintext OAuth token. On multi-user systems (shared CI runners, cloud VMs) this is exploitable.

Fix: Create the file with restricted permissions from the start:

fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
with os.fdopen(fd, "w", encoding="utf-8") as f:
    f.write(json.dumps(payload, ensure_ascii=False, indent=2))

Fix this →

---

2. Non-atomic write causes silent data loss (auth.py:139-143)

path.write_text() is not atomic. If the process is killed or the disk fills up mid-write, auth.json is left truncated/corrupted. On the next run _read_json catches json.JSONDecodeError and silently returns {}, permanently losing all saved OAuth profiles with no error or backup.

Fix: Write to a temp file in the same directory, set permissions, then os.rename() (atomic on POSIX):

import tempfile
fd, tmp = tempfile.mkstemp(dir=path.parent)
try:
    os.chmod(tmp, 0o600)
    with os.fdopen(fd, "w", encoding="utf-8") as f:
        f.write(json.dumps(payload, ensure_ascii=False, indent=2))
    os.replace(tmp, path)
except Exception:
    os.unlink(tmp)
    raise

Fix this →

---

Medium Issues

3. auth use with env-only profiles creates permanently broken CLI state (commands/auth.py:87-98)

def use(profile):
    token, _ = get_env_profile(profile)
    oauth_profile = get_oauth_profile(profile)
    if not token and not oauth_profile:
        raise click.ClickException(...)
    set_active_profile(profile)   # ← persists to auth.json

If a user runs direct auth use --profile ci where ci is defined only via YANDEX_DIRECT_TOKEN_CI, the profile name "ci" is written to ~/.direct-cli/auth.json as the active profile. In any future shell session where that env var is absent (different machine, Docker container, restarted shell without the var), get_credentials raises:

Profile 'ci' is not configured. Use direct auth login --profile ci or set YANDEX_DIRECT_TOKEN_CI.

…on every invocation — including direct auth list and direct auth use --profile ... — because get_active_profile() is called in cli() before any subcommand runs, and then has_refs = True triggers credential resolution. The user is stuck until they manually edit auth.json.

Fix: Either (a) warn at auth use time that env-only profiles are volatile, or (b) skip active-profile credential resolution when invoking auth subcommands (currently auth use/auth list are already invoked under auth, so the ctx.invoked_subcommand != "auth" guard in cli.py correctly skips get_active_profile() — but check whether profile or active_profile in has_refs still triggers resolution for active env profiles).

Actually, re-checking cli.py:91-92: get_active_profile() is already skipped when ctx.invoked_subcommand == "auth", so active_profile = None. If no --profile flag is passed, has_refs won't fire for auth subcommands. The breakage happens with non-auth commands in a session where the env var is gone. The fix is simply to add a warning in auth use when the profile has no OAuth entry.

---

4. Profile suffix collision — different names silently share credentials (auth.py:113-121)

def _profile_suffix(profile: str) -> str:
    for char in profile:
        if char.isalnum():
            normalized.append(char.upper())
        else:
            normalized.append("_")

Profile names "prod-api", "prod.api", and "prod/api" all normalize to PROD_API, so they all read from YANDEX_DIRECT_TOKEN_PROD_API. A user with two profiles "prod-api" (OAuth store) and "prod.api" (env var) will get YANDEX_DIRECT_TOKEN_PROD_API when requesting the env profile, but the list_profiles display would show them as separate entries. No collision detection or warning exists.

This is unlikely to bite most users, but when it does it will be very confusing. Worth at least documenting in the help text or raising an error if two stored OAuth profiles normalize to the same suffix.

---

Minor Notes (not requesting changes, just informational)

• Plaintext token storage: Tokens are stored as plaintext in ~/.direct-cli/auth.json. This is standard practice for CLI tools (cf. ~/.netrc, gh, AWS CLI) and the 0600 permissions are the conventional mitigation, but users on shared machines should be aware.
• Double disk read: get_active_profile() is called in cli() and again inside get_credentials() on the same path. Harmless but redundant.
• No token expiry handling: The TODO comment about refresh_token/expires_in is appropriate to leave; just ensure users know stored OAuth tokens will eventually expire and need auth login again.

---

Summary: Two fixes are essential before merging — the permissions race (#1) and the non-atomic write (#2). Both affect the same 5-line _write_json function and can be addressed together. Issue #3 is worth addressing with a warning at minimum.

[axisrow]

Issues #1 and #2 (security race condition + non-atomic write) fixed in commit 76619ca — now uses tempfile.mkstemp + os.replace for atomic write with 0o600 permissions set before the file is visible.

Issues #3 and #4 are skipped:

• feat: add Bitwarden CLI integration #3 (env-only profile breakage): The guard in cli.py (ctx.invoked_subcommand != "auth") already prevents auth subcommands from resolving the active profile. For non-auth commands in sessions where the env var is gone, the error is raised and reported explicitly — not silent breakage. This is acceptable UX for env-only profiles.
• Fix all linting and formatting issues #4 (profile suffix collision): Edge case acknowledged but cosmetic — low probability in practice and the reviewer explicitly categorized it as informational.

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 2m 40s —— View job

---

PR Review: feat(auth): OAuth profiles with --profile flag and auth subcommands

Tasks:

• Gather context and understand the request
• Read current state of direct_cli/auth.py (after atomic write fix)
• Read direct_cli/commands/auth.py
• Read direct_cli/cli.py and direct_cli/api.py
• Read test files
• Analyze for critical issues
• Post review findings

---

Status of Previously Reported Issues

Fixed in 76619ca — confirmed correct:
The _write_json fix (auth.py:140-151) is solid. tempfile.mkstemp creates the file with mode 0600 atomically, the explicit os.chmod is redundant but harmless, and os.replace ensures the rename is atomic. Both the race condition and data-loss risk are resolved.

---

Remaining Critical Issue

ValueError for profile errors is silently swallowed — wrong credentials used with no warning (cli.py:118-123)

try:
    resolved_token, resolved_login = get_credentials(
        token=token, login=login, profile=profile, ...
    )
    ctx.obj["token"] = resolved_token
    ctx.obj["login"] = resolved_login
except RuntimeError as e:
    raise click.ClickException(str(e))
except ValueError:
    # No token provided — let subcommands fail naturally
    ctx.obj["token"] = token   # ← token is None here
    ctx.obj["login"] = login

get_credentials raises ValueError in two cases:

1. Profile selected but not found (auth.py:410): "Profile 'X' is not configured. Use direct auth login…"
2. No token found at all (auth.py:442): "API token required. Set YANDEX_DIRECT_TOKEN…"

The except ValueError block catches both cases and discards the error message. Case 2 is intentionally deferred (subcommand fails naturally). But case 1 — an explicit profile that's missing — is also silenced.

Scenario A — confusing error message:

$ direct --profile prod campaigns get
Error: API token required. Set YANDEX_DIRECT_TOKEN...

The user gets a generic "no token" error instead of "Profile 'prod' is not configured."

Scenario B — silent wrong-account execution (more dangerous):

$ direct auth use prod   # saves "prod" to auth.json
# prod's entry later disappears (file corruption / manual edit)
# YANDEX_DIRECT_TOKEN=sandbox_token is set in env
$ direct campaigns get
# Silently uses sandbox_token — wrong account, no warning

Step by step: get_credentials(profile="prod") → profile not in store, no env var → ValueError("Profile 'prod' is not configured") → swallowed → ctx.obj["token"] = None → subcommand calls create_client(token=None, profile=None) → get_credentials(None, None) (no profile) → finds YANDEX_DIRECT_TOKEN → returns sandbox token. The user, expecting prod, is operating on the wrong account with no indication.

Fix: Distinguish the two ValueError cases by checking whether a profile was selected:

except RuntimeError as e:
    raise click.ClickException(str(e))
except ValueError as e:
    if profile or active_profile:
        # Profile was explicitly selected but not found — show the error
        raise click.ClickException(str(e))
    # No credential source at all — let subcommands fail with their own error
    ctx.obj["token"] = token
    ctx.obj["login"] = login

Fix this →

---

Verified — Not Issues

• _write_json fix: Correct. mkstemp already creates with 0600; os.replace is atomic on POSIX.
• api.py missing bw_* params: By design — create_client receives an already-resolved token from ctx.obj; get_credentials reads YANDEX_DIRECT_BW_* env vars itself on fallback.
• auth subcommand guard: ctx.invoked_subcommand != "auth" correctly prevents auth list/auth use from triggering the credential-resolution block.

---

Summary: One issue remains before merging. The ValueError swallowing bug in cli.py:120 can cause silent credential fall-through when a profile is configured as active but its token entry is missing — the user silently gets credentials from YANDEX_DIRECT_TOKEN without any warning. The fix is a one-line change to the except clause.

[axisrow]

@claude review. Focus on critical issues: bugs, security vulnerabilities, logical errors, data loss risks, performance problems. Do NOT nitpick style, naming conventions, minor formatting, or subjective preferences — only flag issues that could break functionality or cause real harm in production.

[claude]

Claude finished @axisrow's task in 3m 8s —— View job

---

PR Review: feat(auth): OAuth profiles with --profile flag and auth subcommands

Tasks:

• Gather context and understand the request
• Read current state of direct_cli/cli.py (verify ValueError fix from c7d0931)
• Read current state of direct_cli/auth.py
• Read current state of direct_cli/commands/auth.py
• Read direct_cli/api.py and test files
• Analyze for any remaining critical issues
• Post review findings

---

Status of All Previously Reported Issues

All three critical issues are confirmed fixed and correct:

_write_json atomic write + permissions (76619ca) — ✅ Solid
auth.py:140-151 uses tempfile.mkstemp (creates file with mode 0600 atomically before any write), os.chmod(tmp, 0o600) (redundant but harmless), and os.replace (atomic rename on POSIX). Both the race window and data-loss risks are fully eliminated.

ValueError swallowing (c7d0931) — ✅ Solid
cli.py:120-126 now catches ValueError as e and checks if profile or active_profile: before raising click.ClickException. The explicit-profile case is surfaced; the no-credentials case is still silently deferred to subcommands. Logic is correct.

---

No New Critical Issues Found

Checked all paths thoroughly:

• Double credential resolution (cli() then create_client): subcommands pass the already-resolved ctx.obj["token"] to create_client, which in turn calls get_credentials(token=<resolved>). Since an explicit token always takes priority (if selected_profile and not final_token:), the second resolution is a no-op. No scenario exists where the two resolutions can return different credentials for the same invocation.

• auth subcommand guard: ctx.invoked_subcommand != "auth" correctly prevents active-profile lookup for auth login/auth list/auth use/auth status. None of those subcommands call create_client, so there is no secondary resolution path to worry about.

• list_profiles env scanning: correctly guarded against empty suffixes; case normalization (suffix.lower()) is consistent with _profile_suffix for lookup round-trips.

• PKCE flow: code_verifier is generated and passed to exchange_oauth_code in the same function scope — no state persistence issues.

• Error handling: exchange_oauth_code surfaces HTTP error bodies; op_read/bw_read raise RuntimeError on failure; all bubble up to click.ClickException correctly in command handlers.

• OAuth client ID: DEFAULT_OAUTH_CLIENT_ID is the public app identifier, not a secret — fine to hardcode.

Minor Testing Note (not blocking)

There is no end-to-end CLI-runner test for the c7d0931 fix itself: direct --profile nonexistent campaigns get → Error: Profile 'nonexistent' is not configured. The fix is exercised at the get_credentials unit level (test_profile_does_not_fallback_to_base_env), but not at the cli() → ClickException integration level. Not a blocker, but a useful test to add post-merge.

---

Summary: All previously reported critical and medium issues are correctly fixed. No new critical issues found. This PR is clean from a security, correctness, and data-loss perspective.