[jcvrabo]

Thank you for contributing to the CF CLI! Please read the following:

• Please make sure you have implemented changes in line with the contributing guidelines
• We're not allowed to accept any PRs without a signed CLA, no matter how small.
  If your contribution falls under a company CLA but your membership is not public, expect delays while we confirm.
• All new code requires tests to protect against regressions.
• Contributions must be made against the appropriate branch. See the contributing guidelines
• Contributions must conform to our style guide. Please reach out to us if you have questions.

Note: Please create separate PR for every branch (main and v8) as needed.

Description of the Change

While this PR may not be considered for merging, it intends to support #3621 showing how the current v8 plugin implementation may be refactored in order to fully support v3 calls and prepare the project to completely drop any v2 related code. A main branch PR has not been created unless the pr is deemed worthy and a main pr is requested (such a PR would/should probably already have the deprecated methods fully removed).

Changes of note are:

• It keeps the RPC protocol and plugin implementation unchanged, with the exception of adding an additional method to CliConnection which now returns an initialized cfclient that may be used by newly implemented (or updated) plugins instead of the existing Get* methods (which are now tagged as deprecated.
• The internal implementation of the existing Get* methods make use of v7action.Actor to get the required information, and when methods do not exist, it makes use of the Actor's CloudControllerClient to get the additional data. Because the current Get methods are meant to be deprecated, no new Actor methods were added to fully support the CliConnection Get methods.
• CallCoreCommand makes use of the existing command_parser (used to run commands by main) instead of the legacy command registry and simplifies the output capture which can be retrieved by the plugin.
• It uses the configv3.Config object already initialized at the start to get configuration values instead of reading it again later with the corerepository implementation.
• No V2 code is used, and the legacy command architecture is eligible to be removed (not done in this pr as it falls out of scope and it would add too much clutter to the PR review).

Some caveats:

• Listing org users or space users will always return the isAdmin attribute as false. Supporting this attribute would require calling uaa for every listed user to check if it would have the cloud_controler.admin scope. Seems unnecessary and a high price to pay.
• The domain.name value when listing routes currently returns the full route url as the current action is not returning the domain name. This may be updated to properly support the domain name
• When returning the buildpack for an App, since the legacy model returns a single buildpack, the first buildpack of the app current droplet will be returned. This may be updated to return a comma separated list of buildpacks instead.
• The plugin related commands (install, uninstall, repo, etc.) were not reviewed and still work as before. These may also be reviewed.
• The provided CfClient is created only with the access token, which is only suitable for immediate commands and not for longer processing plugins. For that some changes in the RPC protocol would have to be done so that the refresh token could also be obtained by the plugin. As it is, plugin implementations could handle some refresh method which would call the rpc AccessToken method whenever required.

Why Is This PR Valuable?

It finally drops the dependency of the V2 cc API and allow plugin backward compatibility which may remain unchanged (although they shouldn't) unless they explicitly invoke V2 endpoints. From the CLI scope perspective, it's the next evolving step.

Applicable Issues

#3621

How Urgent Is The Change?

V2 is bound to be fully decommissioned and several foundation installations already have the V2 api disabled. This will allow the v2 api to be fully decommissioned while having the CLI fully compatible (v3 plugin compatibility, if it directly calls v2 endpoints, falls under the responsibility of the plugin maintainers)

Other Relevant Parties

All plugin users/developers

[jcvrabo]

would like to mention that the CVE check failures applies to the current live branch, when those are updated a fork sync will also solve it.

[Gerg]

🤖 AI-Assisted Architectural & Compatibility Review

Thank you for putting together this proof of concept! Migrating the CLI plugin RPC layer from Cloud Controller V2 to V3 is a critical and long-awaited step (#3621).

We conducted an adversarial review and empirical test validation of the proposed changes against origin/v8. While the overall direction of using v7action.Actor and configv3.Config is sound, we identified two blocking launch/lifecycle regressions and several public model mapping discrepancies that would break existing plugins relying on backward compatibility:

Key Findings Summary

1. [Blocker] Offline Plugin Launch (F1): PluginRunner.Run unconditionally dials Cloud Controller before starting the RPC listener, failing with No API endpoint set when no target is configured (plugin/runner/runner.go:79).
2. [Blocker] Lifecycle Service Nil Dereference (F2): Lifecycle commands (uninstall-plugin) create RpcService with nil dependencies; plugin uninstall callbacks calling AccessToken() panic on nil dereference (command/plugin/shared/rpc.go:46).
3. [Should-Fix] Public Model Mapping Regressions:
   • UserEmail() returns username rather than the JWT email claim (plugin/rpc/cli_rpc_server.go:255).
   • GetSpace duplicates security groups bound to both running and staging (plugin/rpc/cli_rpc_server.go:618).
   • Application State casing changes from "started" to "STARTED" (plugin/rpc/mappings.go:381).
   • Role strings change from legacy constants (RoleOrgManager) to v3 enums (organization_manager) (plugin/rpc/mappings.go:118).
   • Unlimited quota limits become 0 instead of -1 due to null unmarshaling (plugin/rpc/mappings.go:189).
   • Domain.Name receives full route URLs rather than domain names (plugin/rpc/mappings.go:462).
   • GetApp returns binding names instead of service instance names (plugin/rpc/mappings.go:471).
   • GetServices drops ServicePlan.Guid (plugin/rpc/mappings.go:84).
   • GetApps reports observed live instances instead of desired scale for stopped apps (plugin/rpc/mappings.go:503).
   • GetApp exposes redacted start command [PRIVATE DATA HIDDEN IN LISTS] and omits environment variables (plugin/rpc/cli_rpc_server.go:331).
4. [Should-Fix] Concurrency Data Race (F4): CallCoreCommand shares an unsynchronized bytes.Buffer across stdout and stderr interceptors (plugin/rpc/cli_rpc_server.go:53).
5. [Should-Fix] Credential Persistence (F19): Token refreshed during RPC AccessToken() is not saved to disk on exit, risking UAA refresh token invalidation (plugin/rpc/cli_rpc_server.go:315).

Detailed inline review comments with citations to current origin/v8 source code and suggested fixes are attached below.

[Gerg]

🤖 AI Review [Blocker]: Plugin launch fails when no API target is configured

• Issue: PluginRunner.Run calls shared.GetNewClientsAndConnectToCF unconditionally before launching the plugin child process. If no CF target is configured in config.Target(), connectToCF returns translatableerror.NoAPISetError, aborting execution with "error connecting to CF: No API endpoint set" before the plugin starts.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/cmd/cmd.go:142 and plugin/rpc/run_plugin.go:10), the CLI starts the RPC listener and launches the plugin binary without connecting over the network. This allows offline plugins, target switchers, and plugins inspecting cliConnection.HasAPIEndpoint() to function.
• Suggested fix: Only attempt connecting to Cloud Foundry when r.config.Target() != "". If untargeted, launch the plugin with actor = nil:

var actor *v7action.Actor
if r.commandUI != nil && r.config.Target() != "" {
    // initialize sharedActor, clients, and actor
}

[jcvrabo]

F1/F2: Fixed. The runner only builds the actor when an API is targeted, so plugins launch without a target. RPC calls that need Cloud Controller then return a "No API endpoint set" error, while config-only calls (HasAPIEndpoint(), …) keep working. Contexts without an actor or parser (the install/uninstall lifecycle and the legacy paths) return explicit errors instead of panicking. AccessToken() without an actor returns the token stored in the config, without refreshing it. This is a deliberate policy: a plugin's CLI-MESSAGE-UNINSTALL hook is meant for cleaning up its own files and notifying outside services, not for using Cloud Controller.

(4eec7efcd)

[Gerg]

🤖 AI Review [Cleanup]: Unused legacy commandregistry.NewDependency initialization

• Issue: deps is allocated and deferred-closed (deps.Config.Close()), but it is never passed to rpc.NewRpcService or referenced anywhere in PluginRunner.Run.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/cmd/cmd.go:142), deps was actively passed to rpc.NewRpcService and legacy command runners.
• Suggested fix: Remove lines 68–71 in plugin/runner/runner.go and drop "code.cloudfoundry.org/cli/v8/cf/commandregistry" from imports to avoid unnecessary disk I/O and legacy package coupling.

[jcvrabo]

Removed. The same unused deps/trace logger was also in command/plugin/shared/rpc.go, and the unused dialTimeout var was in cli_rpc_server.go. Both are gone too, and the new plugin paths no longer import cf/commandregistry.

(a1b335d3e)

[Gerg]

🤖 AI Review [Blocker]: Lifecycle RPC service passes nil dependencies, causing panics

• Issue: NewRpcService is initialized with nil for actor, commandParser, and commandUI. During cf uninstall-plugin, the CLI executes the plugin binary with CLI-MESSAGE-UNINSTALL. If the plugin cleanup routine calls AccessToken(), CallCoreCommand(), or GetApp(), the RPC server dereferences cmd.actor without a nil check and crashes with a nil pointer dereference panic.
• Equivalent behavior in current plugin interface: In origin/v8 (command/plugin/shared/rpc.go:40 and cf/commands/plugin/uninstall_plugin.go:58), the RPC service was supplied with concrete dependencies (deps.RepoLocator, deps.Config, rpc.NewCommandRunner()).
• Suggested fix: Add defensive nil guards to all CliRpcCmd methods in plugin/rpc/cli_rpc_server.go:

if cmd.actor == nil {
    return errors.New("Cloud Controller client not initialized")
}

[jcvrabo]

F1/F2: Fixed. The runner only builds the actor when an API is targeted, so plugins launch without a target. RPC calls that need Cloud Controller then return a "No API endpoint set" error, while config-only calls (HasAPIEndpoint(), …) keep working. Contexts without an actor or parser (the install/uninstall lifecycle and the legacy paths) return explicit errors instead of panicking. AccessToken() without an actor returns the token stored in the config, without refreshing it. This is a deliberate policy: a plugin's CLI-MESSAGE-UNINSTALL hook is meant for cleaning up its own files and notifying outside services, not for using Cloud Controller.

(4eec7efcd)

[Gerg]

🤖 AI Review [Should-Fix]: Data race in OutputInterceptor.Write during core command execution

• Issue: CallCoreCommand instantiates a single cmd.outputBucket = &bytes.Buffer{} shared between stdout and stderr OutputInterceptors without mutex synchronization. Core v7 commands that emit to stdout and stderr concurrently from separate goroutines (e.g. streaming logs while logging status warnings) trigger data races on bytes.Buffer.Write, leading to slice index panics or buffer corruption.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/terminal/tee_printer.go:61), commands were legacy V2 commands where output was routed through terminal.TeePrinter.
• Suggested fix: Wrap buffer writes in a thread-safe synchronized buffer:

type synchronizedBuffer struct {
    mu  sync.Mutex
    buf bytes.Buffer
}
func (sb *synchronizedBuffer) Write(p []byte) (int, error) {
    sb.mu.Lock()
    defer sb.mu.Unlock()
    return sb.buf.Write(p)
}

[jcvrabo]

Fixed. Captured output now goes through a mutex-guarded buffer shared by the stdout and stderr interceptors. CallCoreCommand/DisableTerminalOutput/GetOutputAndReset are also serialized, because net/rpc serves each plugin connection on its own goroutine and core commands mutate the shared config. Added a -race test that writes to both streams concurrently.

Note: legacy TeePrinter wasn't synchronized either (it used a single stdout stream, so concurrent writes were rarer). With v7 commands writing to both stdout and stderr from several goroutines it became easy to hit, so the buffer is now mutex-protected.

Also note that GetOutputAndReset is a misnomer: since 2015 it hasn't reset anything upstream. It returns the latest core command's output, which is only cleared when the next core command starts. The behavior (and the name, which plugins call by string) is kept to match upstream. As upstream does, the buffer is now created with the RPC service and cleared at the start of each CallCoreCommand.

(3a1685937)

[Gerg]

🤖 AI Review [Should-Fix]: UserEmail returns username claim instead of user email

• Issue: cmd.cliConfig.CurrentUser() only decodes user_name into user.Name. In enterprise UAA, LDAP, or SAML setups, user_name is an account handle (e.g. jdoe), while the email address is stored in the "email" claim (e.g. jdoe@example.com). Returning user.Name passes the username rather than the email.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/configuration/coreconfig/config_repository.go:294 and cf/configuration/coreconfig/access_token.go:16), UserEmail() decoded the JWT token and returned the "email" claim from TokenInfo.Email.
• Suggested fix: Decode the "email" claim directly from the JWT access token using utiljwt.ParseUnverified:

claims, err := utiljwt.ParseUnverified(cmd.cliConfig.AccessToken())
if err == nil {
    if email, ok := claims["email"].(string); ok {
        *retVal = email
        return nil
    }
}
*retVal = ""
return nil

[jcvrabo]

Fixed. configv3.User now carries the email claim (decoded through utiljwt.ParseUnverified in decodeUserFromJWT, after syncing with upstream), and UserEmail() returns it. The test fixture now uses different values for username and email, so this can't regress silently.

Note: this matches upstream, which decoded the email claim from the stored access token (coreconfig.NewTokenInfo).

(e2ed492d3)

[Gerg]

🤖 AI Review [Should-Fix]: Route.Domain.Name mapped to full route URL instead of domain name

• Issue: populateAppModel (line 462) and populateAppsModel (line 519) set Domain.Name = route.URL. route.URL in CAPI v3 contains the host and domain (e.g. myapp.apps.example.com). Plugins that construct route URLs using fmt.Sprintf("%s.%s", route.Host, route.Domain.Name) produce myapp.myapp.apps.example.com.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/commands/application/app.go:255), Domain.Name was the actual domain name (e.g. apps.example.com) and Host was myapp.
• Suggested fix: Extract the domain name by trimming the host prefix from route.URL:

domainName := route.URL
if route.Host != "" {
    domainName = strings.TrimPrefix(domainName, route.Host+".")
}

And assign domainName to Domain.Name.

[jcvrabo]

Fixed. Trimming only the host prefix would still leave paths (/api) and TCP ports (:1024) on the domain, so I exported the existing v7action helper that cf routes already uses (DomainNameFromRouteURL: host prefix, path suffix, port suffix) and used it for GetApp and GetApps. Tests cover HTTP, path, and TCP routes.

(52a4f5039)

[Gerg]

🤖 AI Review [Should-Fix]: GetApp returns credential binding name instead of service instance name

• Issue: populateAppModel maps binding.Name to ServiceSummary.Name. binding.Name is the name of the service credential binding, not the service instance. For an instance named my-db with a binding db-reader (or an unnamed binding), Name becomes "db-reader" or "".
• Equivalent behavior in current plugin interface: In origin/v8 (cf/commands/application/app.go:270), ServiceSummary.Name contained the service instance name (service.Name).
• Suggested fix: Query service instance details or include service instance names when querying credential bindings, and map the service instance name to ServiceSummary.Name.

[jcvrabo]

Fixed. GetApp now resolves the bound instances with a single /v3/service_instances?guids=… call and maps Name to the service instance name. Results are de-duplicated per instance, because v3 allows several bindings to the same instance.

(2a1dc13e1)

[Gerg]

🤖 AI Review [Should-Fix]: GetApps reports observed rather than desired instance count

• Issue: webProcess.TotalInstanceCount() counts entries in InstanceDetails. For a stopped app configured for 3 instances, InstanceDetails is empty, so TotalInstances returns 0.
• Equivalent behavior in current plugin interface: In origin/v8 (cf/commands/application/apps.go:131), TotalInstances reported the configured desired instance count (app.InstanceCount), independently of live running instances.
• Suggested fix: Populate TotalInstances from the web process configured scale (webProcess.Instances.Value), matching what populateAppModel already does for GetApp:

if webProcess != nil {
    model.TotalInstances = webProcess.Instances.Value
    model.RunningInstances = webProcess.HealthyInstanceCount()
}

[jcvrabo]

Fixed. TotalInstances now comes from the web process's desired instances (the same source GetApp uses for InstanceCount), so a stopped app scaled to 3 reports 3/0. Apps without a web process now fall back to their first process (in GetApp too) instead of reporting zeros.

(3179ffa1e)

[Gerg]

🤖 AI Review [Note]: CfClient() cannot refresh expired tokens during long-running plugin operations

• Issue: cfconfig.Token(token, "") supplies an empty refresh token. Plugins executing long tasks (such as database migrations, bulk exports, or deployment monitoring) will fail with oauth2: token expired once the access token expires.
• Equivalent behavior in current plugin interface: In origin/v8 (plugin/cli_connection.go:278), plugins making direct API calls repeatedly invoked cliConnection.AccessToken(), which auto-refreshed expired tokens via the RPC server.
• Suggested fix: Add a RefreshToken RPC method on CliRpcCmd so cliConnection.CfClient() can obtain both tokens:

opts := []cfconfig.Option{
    cfconfig.Token(token, refreshToken),
    cfconfig.UserAgent(userAgent),
}

Alternatively, document in godoc that CfClient() is scoped to short-lived operations.

[jcvrabo]

Documented on CliConnection.CfClient(): the returned client is good until the current access token expires, and long-running plugins should call CfClient() again (each call goes through the AccessToken RPC, which refreshes when needed). I chose not to expose the refresh token: the legacy plugin API never did, and letting go-cfclient refresh on its own could rotate the UAA refresh token behind the CLI's back and log the user out (the F19 problem). Adding token refresh to CfClient() (a RefreshToken RPC, or preferably an RPC-backed oauth2.TokenSource, which needs a go-cfclient option that doesn't exist yet) is left for a future improvement PR rather than this one.

(692925078)

[Gerg]

🤖 AI Review [Pre-existing]: Global flags preceding a plugin command prevent plugin execution

• Issue: Checking os.Args[1] means commands like cf -v <plugin-command> evaluate FindPluginByCommand("-v"), which returns false and falls through to the unknown command handler.
• Equivalent behavior in current plugin interface: In origin/v8 (util/plugin/plugin.go:34), IsPluginCommand also inspected only osArgs[0].
• Suggested fix: Although pre-existing, this can be improved by parsing the plugin command token from the args or tracking it separately from migration blockers.

[jcvrabo]

Fixed here, even though it's pre-existing (util/plugin.IsPluginCommand also only looked at osArgs[0]). Leading arguments starting with -/-- are now treated as global options and skipped to find the command (runner.CommandArgs). The plugin receives the command and all arguments after it, so cf -v my-plugin-cmd --opt x runs my-plugin-cmd with --opt x. Unknown commands now also report the actual command name instead of the flag.

(69b674746)

[jcvrabo]

All inline review threads are addressed (replies on each thread). Beyond the review, I synced the branch with upstream/v8 (this fixes check-cves), and fixed several things the review didn't cover:

• Build: plugin/pluginfakes/fake_cli_connection.go wasn't regenerated after adding CfClient(), so go build ./... failed for anyone importing pluginfakes (c9c2b8aaa). plugin/cli_connection.go was also re-indented with tabs.
• Legacy fallback: The last three v7 commands that still fell back to the legacy main (list-plugin-repos, repo-plugins, remove-plugin-repo) are now ported to v7 (pluginaction + configv3). Plugins calling them through CliCommand run them in-process again, as upstream did through the legacy registry, instead of re-entering the legacy main with the host's arguments. (eba7304c3) With that, nothing triggers the legacy-main fallback anymore, so it's removed (62b1d947c): util/command_parser no longer calls cf/cmd.Main, and cf/cmd (with the legacy command registry) is no longer linked into the binary. The legacy cf/... code stays in the repo for a separate removal. This also removes the path through which a plugin's CliCommand re-entered the legacy main with the host's os.Args (re-launching the plugin).
• Warnings on stdout: Get* RPC methods used to print Warning: … on stdout. Legacy suppressed all output for these calls, so this would have corrupted plugin stdout. Warnings now go to stderr through UI.DisplayWarnings, the same as v7 commands. (12a026907)
• Instance state in GetApp().Instances had the same casing regression as the app state (RUNNING vs running) and is lowercased too (ceefa4b8d).
• Lint: Marking the Get* methods deprecated makes staticcheck flag the two test plugins that still call them on purpose. Those 15 calls now carry //nolint:staticcheck with a reason. stopPlugin explicitly ignores the Kill/Wait errors. With this, the branch adds no lint issues compared to upstream/v8 (and it removes 4). (4dfbdda42)

Compatibility notes (worth reading for plugin authors):

1. CliCommand output is now v7 output. Upstream's CallCoreCommand ran the legacy v6 commands, so plugins received v6 output. Core commands called from plugins now produce v7 output (different tables, headers and wording). Plugins that scrape CliCommand output may need changes; the structured alternatives are the Get* models or CfClient(). Stdout and stderr are still captured together, as before.
2. Install/uninstall callbacks have no Cloud Controller access. In CLI-MESSAGE-UNINSTALL, Get* and CliCommand return errors, and AccessToken() returns the stored token without refreshing it. Upstream gave these callbacks full access through the legacy registry. This is deliberate: the hook is for cleaning up the plugin's own files and notifying outside services.
3. CfClient() on the public CliConnection interface breaks third-party code that implements that interface (custom fakes), and adds github.com/cloudfoundry/go-cfclient/v3 v3.0.0-alpha.20 (an alpha release, which is stable and production ready... I have requested already for a final release and even though it was agreed, it is still waiting) to every plugin's dependency graph. The client is valid only until the current access token expires (documented on the method).

Proposed for follow-up PRs (out of scope here):

• Token refresh inside CfClient() (a RefreshToken RPC or, preferably, an RPC-backed oauth2.TokenSource, which needs a go-cfclient option).
• Blocking plugin lifecycle commands (install-plugin/uninstall-plugin) from CliCommand, so a running plugin can't replace or delete installed plugins, itself included.
• Applying leading global options (e.g. -v verbose request logging) to plugin runs. They are currently only skipped to find the command.
• Removing the now-unreachable legacy cf/... code.

Still not populated (known gaps): GetApp DetectedStartCommand, PackageUpdatedAt and StagingFailedReason; GetApps route Domain.OwningOrganizationGuid and Domain.Shared. GetOrgUsers/GetSpaceUsers IsAdmin is always false, which already matched legacy.

Testing: unit tests for every change (RED → GREEN), -race on plugin/rpc and plugin/runner, and smoke runs of a built cf against a fixture plugin and a local plugin repository. Not run: the CF-backed integration suites.