Add access controls to bb apps - #293
Conversation
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
REQUEST_CHANGES. A fresh static review of the exact three-dot comparison c5058ae...532017e found one blocking consent risk and one non-blocking discoverability gap in the new access-policy command: omitting every viewer silently clears restricted access, and the CLI does not explain which viewer identity to supply or where to find it. The project code-review and user wes-review skills were explicitly loaded and applied as distinct Engineering passes. Final self-check covered the access help, get, and full-policy replacement flows; accessibility; i18n/localization completeness; navigation and consent guards; bounded network waits, error and never-completes behavior, stateless lifecycle and race behavior; test honesty; design-system and project rules; duplicate and overlapping findings; and concrete evidence plus user effect for blocking severity. The supplied GitHub evidence was inspected: seven reported checks completed successfully, while Tauri Rust checks (macOS) and Frontend checks and unit tests were in progress; required checks independently govern merge readiness.
Deterministic publication result: 1 blocking and 1 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
morgmart
left a comment
There was a problem hiding this comment.
🤖 Automated code review
COMMENT. A fresh static review of the full exact three-dot comparison c5058ae...b89c2eb found no publishable findings. The prior restricted-viewer clearing risk is fixed by affirmative --clear-viewers consent plus pre-credential rejection of accidental omission, and viewer identity discovery is now connected to the exact caller subject from bb apps list --json; both fixes have discriminating coverage. The project code-review and user wes-review skills were explicitly loaded and applied as distinct Engineering passes. Final self-check covered access discovery, get, organization and restricted replacement, explicit clearing, invalid and conflicting arguments, environment selection, authorization and control-plane errors, bounded waits and never-completes behavior, retry/idempotence, lifecycle and concurrency, accessibility, i18n/localization, navigation and consent guards, test honesty, design-system and project rules, duplicate overlap, and concrete evidence plus user effect for blocking severity. No unsupported implementation finding survived. Atlas retains one product-policy escalation about the original-owner and approved-publisher access model, so the recommendation is COMMENT rather than APPROVE. All ten supplied GitHub check runs completed successfully; the supplied combined commit status remains pending with no status contexts, so required checks still independently govern merge readiness.
Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.
Summary
bb appscan manage the app lifecycle, but it cannot inspect or update who may view a deployed app. This adds:bb apps access get <APP_ID> [--environment <ENVIRONMENT>]bb apps access set <APP_ID> --visibility organization|restricted [--viewer <IDENTITY>...] [--clear-viewers] [--environment <ENVIRONMENT>]These commands map to the control plane GET and PUT access endpoints through the existing allowlisted client.
setreplaces the full policy. Restricted policies require at least one--vieweror an explicit--clear-viewersconfirmation, while the owner and approved publishers remain effective viewers. Viewer help points to the exact case-sensitivecallersubject returned bybb apps list --json. The control plane remains authoritative for identity validation and owner-only mutation authorization, and responses are preserved unchanged.Related issue
N/A. No matching open issue or PR found.
Testing
At commit
b89c2ebe:cd bb-cli && source ../bin/activate-hermit && just check(153 unit, 81 bb E2E, 22 CLI E2E, 6 acceptance tests)Live access mutation was not run because the local Builderlab CLI session remains intentionally logged out. The request and response contract was verified against the live control-plane contract and current Compose source, then covered with mocked client and process tests.