Skip to content

Atomic CLI related leftovers around name based lookups - #3798

Open
noise64 wants to merge 11 commits into
mainfrom
cli-id-vs-name-cleanup
Open

Atomic CLI related leftovers around name based lookups#3798
noise64 wants to merge 11 commits into
mainfrom
cli-id-vs-name-cleanup

Conversation

@noise64

@noise64 noise64 commented Aug 31, 2026

Copy link
Copy Markdown
Contributor
  • resolves GOL-327
  • adds exact natural-key registry lookups for accounts, plugins, agent secrets, retry policies, security schemes, and domain registrations
  • switches CLI resource resolution from list-and-search to generated atomic client methods while keeping ID-based mutations
  • resolves account-qualified application/environment references atomically and supports manifest-driven creation for explicit accounts while keeping explicit references lookup-only
  • adds consistent --account <EMAIL> / --account-id <UUID> scope to account, card, and plugin commands
  • supports plugin name/version and explicit --id identities, with documented conflicts and explicit-account list semantics
  • removes retired component-plugin parser/handler scaffolding and updates ignored legacy tests with the retired-workflow reason
  • rewrites current CLI plugin documentation and the plugin-management skill around registry commands and declarative manifests
  • adds parser, SQLite/PostgreSQL repository, and generated-client integration coverage
  • regenerates OpenAPI, REST API reference docs, and skill-derived how-to guides

@netlify

netlify Bot commented Aug 31, 2026

Copy link
Copy Markdown

Deploy Preview for golemcloud canceled.

Name Link
🔨 Latest commit eb79b5d
🔍 Latest deploy log https://app.netlify.com/projects/golemcloud/deploys/6aa280ed0f82ae0008929bed

@github-actions

github-actions Bot commented Aug 31, 2026

Copy link
Copy Markdown

📖 Docs preview: https://docs-b33ios00f-golem-cloud.vercel.app

Built from commit eb79b5ddff386bc7a6f67cc361757653ed3f1ef3 by docs.yaml.

# Conflicts:
#	cli/golem-cli/src/command.rs
#	cli/golem-cli/src/command_handler/account.rs
#	golem-registry-service/tests/repo/mod.rs
#	golem-registry-service/tests/repo/postgres.rs
#	golem-registry-service/tests/repo/sqlite.rs
@noise64 noise64 changed the title Add atomic CLI resource lookups Atomic CLI related leftovers around name based lookups Sep 9, 2026
noise64 and others added 3 commits September 9, 2026 16:46
#3824 added `leadership` to golem_shard_manager::RunDetails; #3840's
local_server_system_memory_override test constructs RunDetails without it,
so the golem lib test failed to compile (a semantic conflict that landed on
main). the test only exercises memory-config plumbing, so leadership: None.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S5f7dgMarefJrfRRd9Xmww
@noise64
noise64 marked this pull request as ready for review September 10, 2026 10:01
@noise64
noise64 requested a review from a team September 10, 2026 10:01

vigoo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Agent review on behalf of @vigoo:

[P2] Natural-key resource lookups require an extra parent-environment permission

The new secret, retry-policy, domain, and security-scheme lookups first call EnvironmentService::get(..., auth), which requires EnvironmentVerb::View, before checking permission to view the requested resource.

Locations: agent secrets, retry policies, domains, and security schemes.

For example, if Alice shares permission to view one secret with Bob without granting environment-view permission, GET /v1/agent-secrets/<secret-id> succeeds, but GET /v1/envs/<environment-id>/agent-secrets/by-path?path=foo&path=bar returns 404. The existing ID lookup loads ownership metadata and checks the secret permission directly; the new lookup rejects Bob at the parent check despite his valid resource grant.

Suggested fix: load the resource and ownership metadata without imposing parent-view permission, then authorize the resource directly, as the new plugin lookup does. Add positive granular-sharing tests for these endpoints, not just owner success and unrelated-user rejection.

[P2] --account <EMAIL> requires broader permissions than --account-id

The new account-scope resolver resolves email through get_account_by_email, which requires AccountVerb::View. Supplying an account ID skips that check and proceeds to the resource operation.

The PR’s plugin integration test grants only account.plugin(alice@example.com) @ bob@example.com : view : my-plugin. With that permission, golem plugin get my-plugin 1.0.0 --account-id <alice-account-id> succeeds, but golem plugin get my-plugin 1.0.0 --account alice@example.com fails during account resolution, before the plugin lookup. The advertised email/ID scope alternatives therefore behave differently under least-privilege sharing.

Suggested fix: resolve account-qualified resource identities within a resource-authorized server lookup rather than fetching the full account DTO first. Keep authorization on the full account endpoint, and extend the granular plugin-sharing test through the CLI email form.

Both findings are based on tracing the authorization paths; live two-account reproductions were not run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants