starred/App-Store-Connect-CLI

Fork 0

mirror of https://github.com/rudrankriyam/App-Store-Connect-CLI.git synced 2026-04-25 07:35:48 +03:00

[GH-ISSUE #525] Strict auth UX: accept common truthy values (yes/y) or document ParseBool-only behavior #148

New issue

Closed

opened 2026-02-26 21:33:47 +03:00 by kerem · 3 comments

kerem commented

2026-02-26 21:33:47 +03:00

Owner

Originally created by @rudrankriyam on GitHub (Feb 14, 2026).
Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/525

Summary

ASC_STRICT_AUTH currently uses strconv.ParseBool, so values like yes/y are ignored (treated as disabled).

Why this matters

Many CLI users expect yes to behave like true in env flags. Silent fallback to disabled strict auth can weaken expected protections.

Current behavior

internal/cli/shared/shared.go:

strictAuthEnabled() parses env with strconv.ParseBool.
Invalid values return false without warning.

Expected behavior

One of:

Accept broader truthy/falsey values (yes/no, y/n, on/off), OR
Keep strict parser but emit a warning for invalid value and document accepted values clearly.

Reproduction

export ASC_STRICT_AUTH=yes
# mixed credential sources still warn instead of failing hard

Detailed implementation plan

Decide parser policy (expanded values vs strict + warning).
Implement parser behavior in strictAuthEnabled().
Add tests for accepted values and invalid-value handling.
Update docs/env var table with explicit accepted forms.

Acceptance criteria

Strict auth env behavior is predictable and documented.
Invalid values are not silently ambiguous.
Tests cover parser behavior comprehensively.

cc @cursor please implement this end-to-end.

Originally created by @rudrankriyam on GitHub (Feb 14, 2026). Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/525 ## Summary `ASC_STRICT_AUTH` currently uses `strconv.ParseBool`, so values like `yes`/`y` are ignored (treated as disabled). ## Why this matters Many CLI users expect `yes` to behave like true in env flags. Silent fallback to disabled strict auth can weaken expected protections. ## Current behavior `internal/cli/shared/shared.go`: - `strictAuthEnabled()` parses env with `strconv.ParseBool`. - Invalid values return false without warning. ## Expected behavior One of: 1. Accept broader truthy/falsey values (`yes/no`, `y/n`, `on/off`), OR 2. Keep strict parser but emit a warning for invalid value and document accepted values clearly. ## Reproduction ```bash export ASC_STRICT_AUTH=yes # mixed credential sources still warn instead of failing hard ``` ## Detailed implementation plan - [ ] Decide parser policy (expanded values vs strict + warning). - [ ] Implement parser behavior in `strictAuthEnabled()`. - [ ] Add tests for accepted values and invalid-value handling. - [ ] Update docs/env var table with explicit accepted forms. ## Acceptance criteria - [ ] Strict auth env behavior is predictable and documented. - [ ] Invalid values are not silently ambiguous. - [ ] Tests cover parser behavior comprehensively. cc @cursor please implement this end-to-end.

kerem

2026-02-26 21:33:47 +03:00

closed this issue
added the
enhancement
label

kerem commented

2026-02-26 21:33:48 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

@cursor please implement this issue end-to-end using the checklist and acceptance criteria.

@rudrankriyam commented on GitHub (Feb 14, 2026): @cursor please implement this issue end-to-end using the checklist and acceptance criteria.

kerem commented

2026-02-26 21:33:48 +03:00

Author

Owner

@cursor[bot] commented on GitHub (Feb 14, 2026):

Implemented end-to-end on cursor/strict-auth-value-handling-27f3 and pushed as commit 611b04f.

Parser policy chosen: expanded strict-auth env parsing (option 1), plus explicit warning on invalid values (so invalid input is no longer silent).

func strictAuthEnabled() bool {
	if strictAuth {
		return true
	}
	value := strings.TrimSpace(os.Getenv(strictAuthEnvVar))
	if value == "" {
		return false
	}
	switch strings.ToLower(value) {
	case "1", "t", "true", "yes", "y", "on":
		return true
	case "0", "f", "false", "no", "n", "off":
		return false
	default:
		fmt.Fprintf(
			os.Stderr,
			"Warning: invalid %s value %q (expected true/false, 1/0, yes/no, y/n, or on/off); strict auth disabled\n",
			strictAuthEnvVar,
			value,
		)
		return false
	}
}

Tests added/updated (comprehensive parser coverage):
- TestStrictAuthEnabled_EnvTruthyValues
- TestStrictAuthEnabled_EnvFalseyValues
- TestStrictAuthEnabled_InvalidValueWarnsAndDisables
- TestCheckMixedCredentialSourcesStrictAuthEnvErrors (proves ASC_STRICT_AUTH=yes enforces strict behavior)
```
// ... new strict-auth env tests ...
```
Docs/help updated with accepted forms:
- README.md strict-auth env docs
- AGENTS.md env var table entry
- internal/cli/auth/auth.go long help text

Validation evidence:

Reproduced failure first (new tests failed before fix):
go test ./internal/cli/shared -run "TestStrictAuthEnabled_|TestCheckMixedCredentialSourcesStrictAuthEnvErrors" (failed on yes/y/on and missing invalid warning).
After implementation: same targeted tests passed.
Full checks passed:
- make format
- make lint
- make test

@cursor[bot] commented on GitHub (Feb 14, 2026): Implemented end-to-end on `cursor/strict-auth-value-handling-27f3` and pushed as commit `611b04f`. - **Parser policy chosen:** expanded strict-auth env parsing (option 1), plus **explicit warning on invalid values** (so invalid input is no longer silent). ```505:526:internal/cli/shared/shared.go func strictAuthEnabled() bool { if strictAuth { return true } value := strings.TrimSpace(os.Getenv(strictAuthEnvVar)) if value == "" { return false } switch strings.ToLower(value) { case "1", "t", "true", "yes", "y", "on": return true case "0", "f", "false", "no", "n", "off": return false default: fmt.Fprintf( os.Stderr, "Warning: invalid %s value %q (expected true/false, 1/0, yes/no, y/n, or on/off); strict auth disabled\n", strictAuthEnvVar, value, ) return false } } ``` - **Tests added/updated (comprehensive parser coverage):** - `TestStrictAuthEnabled_EnvTruthyValues` - `TestStrictAuthEnabled_EnvFalseyValues` - `TestStrictAuthEnabled_InvalidValueWarnsAndDisables` - `TestCheckMixedCredentialSourcesStrictAuthEnvErrors` (proves `ASC_STRICT_AUTH=yes` enforces strict behavior) ```397:495:internal/cli/shared/shared_test.go // ... new strict-auth env tests ... ``` - **Docs/help updated with accepted forms:** - `README.md` strict-auth env docs - `AGENTS.md` env var table entry - `internal/cli/auth/auth.go` long help text Validation evidence: - Reproduced failure first (new tests failed before fix): `go test ./internal/cli/shared -run "TestStrictAuthEnabled_|TestCheckMixedCredentialSourcesStrictAuthEnvErrors"` (failed on `yes/y/on` and missing invalid warning). - After implementation: same targeted tests passed. - Full checks passed: - `make format` - `make lint` - `make test` <p><a href="https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/528"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/view-pr-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/view-pr-light.png"><img alt="View PR" width="86" height="28" src="https://cursor.com/assets/images/view-pr-light.png"></picture></a></p> <p><a href="https://cursor.com/background-agent?bcId=bc-0bd4311d-2943-4fa7-b750-62126c5b3ff3"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> <a href="https://cursor.com/agents?id=bc-0bd4311d-2943-4fa7-b750-62126c5b3ff3"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a></p>

kerem commented

2026-02-26 21:33:48 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

Fixed in #528 (merged to main).

@rudrankriyam commented on GitHub (Feb 14, 2026): Fixed in #528 (merged to main).

kerem referenced this issue

2026-02-26 21:34:35 +03:00

[PR #148] [MERGED] Add TestFlight 'What to Test' notes (betaBuildLocalizations) support #328