add Go unit tests for retrieveToken and vcs URL rewriting

Covers the two pieces of novel backend logic added in the recent port
batch (PR 5169 + PR 5195).

auth_test.go:
  - TestRetrieveToken — success case. Mirrors TestVerifySession's mock
    plumbing (setupHTTPTest, sqlmock, InitStratosAuthService) to stub a
    signed-session user_id/exp and verify the handler returns an
    AuthTokenEnvelope containing the decrypted TokenRecord.
  - TestRetrieveTokenNoSessionDate — error path. Omits "exp" from the
    session so GetSessionInt64Value fails, and verifies the handler
    writes an error envelope (status:"error", data:null) rather than
    propagating the error to echo.

vcs.go:
  - Extracted the access-token URL rewrite logic from Create() into
    vcsCmd.repoWithToken(repo). Create() now delegates to it. Pure
    refactor — no behavioural change — done so the rewrite can be
    unit-tested without shelling out to a real git binary.

vcs_test.go (new file, six tests):
  - TestRepoWithToken_NoTokenReturnsURLUnchanged — no-op when token empty
  - TestRepoWithToken_EmbedsTokenAsBasicAuth — PAT injected as
    x-access-token basic-auth userinfo
  - TestRepoWithToken_InvalidURLReturnsError — malformed URL surfaces
    a wrapped parse error
  - TestRepoWithToken_SpecialCharactersInTokenEscaped — tokens
    containing @, :, / must be percent-encoded so they don't prematurely
    terminate the userinfo section. This is the class of bug that would
    silently point git at the wrong host.
  - TestGetVCS_ReturnsFreshCopyNotPrototype — confirms GetVCS() returns
    a copy of the package-level vcsGit prototype rather than the
    prototype itself, so concurrent callers can't race on a mutated
    accessToken field. Directly exercises the concurrency fix that
    deviated from upstream PR #5195's GetVCS implementation.
  - TestGetVCS_WithAccessTokenOption — confirms the functional-options
    pattern actually applies the token to the returned struct and
    preserves the prototype's other fields.

Author: norman-abramovitz

src/jetstream/auth_test.go

@@ -1136,3 +1136,107 @@ func TestVerifySessionExpired(t *testing.T) {
 	})
 
 }
+
+func TestRetrieveToken(t *testing.T) {
+	t.Parallel()
+
+	Convey("Test retrieve UAA token for current session", t, func() {
+
+		req := setupMockReq("GET", "", map[string]string{
+			"username": "admin",
+			"password": "changeme",
+		})
+		res, _, ctx, pp, db, mock := setupHTTPTest(req)
+		defer db.Close()
+
+		if e := pp.InitStratosAuthService(api.Remote); e != nil {
+			log.Fatalf("Could not initialise auth service: %v", e)
+		}
+
+		// Mirror TestVerifySession: stub a signed-session user_id and
+		// exp so the handler's session lookups succeed without a real
+		// login round-trip.
+		sessionValues := make(map[string]interface{})
+		sessionValues["user_id"] = mockUserGUID
+		sessionValues["exp"] = time.Now().Add(time.Hour).Unix()
+
+		if errSession := pp.setSessionValues(ctx, sessionValues); errSession != nil {
+			t.Error(errors.New("unable to mock/stub user in session object"))
+		}
+
+		mockTokenGUID := "mock-token-guid"
+		encryptedUAAToken, _ := crypto.EncryptToken(pp.Config.EncryptionKeyInBytes, mockUAAToken)
+
+		// The handler runs VerifySession before the token lookup;
+		// VerifySession reads from the tokens table too.
+		expectedTokensRow := testutils.GetEmptyTokenRows("disconnected", "user_guid", "linked_token").
+			AddRow(mockTokenGUID, encryptedUAAToken, encryptedUAAToken, mockTokenExpiry, "oauth", "", true)
+		mock.ExpectQuery(selectAnyFromTokens).
+			WithArgs(mockUserGUID).
+			WillReturnRows(expectedTokensRow)
+
+		// GetUAATokenRecord's FindAuthToken call
+		rs := testutils.GetEmptyTokenRows("disconnected", "user_guid", "linked_token").
+			AddRow(mockTokenGUID, encryptedUAAToken, encryptedUAAToken, mockTokenExpiry, "oauth", "", true)
+		mock.ExpectQuery(findUAATokenSQL).
+			WillReturnRows(rs)
+
+		if err := pp.retrieveToken(ctx); err != nil {
+			t.Error(err)
+		}
+
+		header := res.Header()
+		contentType := header.Get("Content-Type")
+
+		Convey("Should have expected contentType", func() {
+			So(contentType, ShouldContainSubstring, "application/json")
+		})
+
+		Convey("Should return an ok envelope with token data", func() {
+			So(res, ShouldNotBeNil)
+			body := strings.TrimSpace(res.Body.String())
+			So(body, ShouldContainSubstring, `"status":"ok"`)
+			So(body, ShouldContainSubstring, `"token_guid":"mock-token-guid"`)
+			So(body, ShouldContainSubstring, `"auth_token":"`+mockUAAToken+`"`)
+			So(body, ShouldContainSubstring, `"refresh_token":"`+mockUAAToken+`"`)
+		})
+	})
+}
+
+func TestRetrieveTokenNoSessionDate(t *testing.T) {
+	t.Parallel()
+
+	Convey("Test retrieveToken with missing session exp", t, func() {
+
+		req := setupMockReq("GET", "", map[string]string{
+			"username": "admin",
+			"password": "changeme",
+		})
+		res, _, ctx, pp, db, _ := setupHTTPTest(req)
+		defer db.Close()
+
+		if e := pp.InitStratosAuthService(api.Local); e != nil {
+			log.Fatalf("Could not initialise auth service: %v", e)
+		}
+
+		// Intentionally omit "exp" so the first session read fails.
+		sessionValues := make(map[string]interface{})
+		sessionValues["user_id"] = mockUserGUID
+
+		if errSession := pp.setSessionValues(ctx, sessionValues); errSession != nil {
+			t.Error(errors.New("unable to mock/stub user in session object"))
+		}
+
+		err := pp.retrieveToken(ctx)
+		Convey("Should not propagate the error (handler writes error envelope instead)", func() {
+			So(err, ShouldBeNil)
+		})
+
+		Convey("Should return an error envelope with no token data", func() {
+			So(res, ShouldNotBeNil)
+			body := strings.TrimSpace(res.Body.String())
+			So(body, ShouldContainSubstring, `"status":"error"`)
+			So(body, ShouldContainSubstring, `"data":null`)
+		})
+	})
+}

src/jetstream/plugins/cfapppush/vcs.go

@@ -55,23 +55,35 @@ type vcsCmd struct {
 }
 
 func (vcs *vcsCmd) Create(skipSSL bool, dir string, repo string, branch string) error {
-	if len(vcs.accessToken) > 0 {
-		repoURL, err := url.Parse(repo)
-		if err != nil {
-			return fmt.Errorf("could not parse repo URL for authenticated clone: %w", err)
-		}
-		repoURL.User = url.UserPassword("x-access-token", vcs.accessToken)
-		repo = repoURL.String()
+	authenticatedRepo, err := vcs.repoWithToken(repo)
+	if err != nil {
+		return err
 	}
 
 	for _, cmd := range vcs.createCmd {
-		if err := vcs.run(".", cmd, "sslVerify", strconv.FormatBool(!skipSSL), "dir", dir, "repo", repo, "branch", branch); err != nil {
+		if err := vcs.run(".", cmd, "sslVerify", strconv.FormatBool(!skipSSL), "dir", dir, "repo", authenticatedRepo, "branch", branch); err != nil {
 			return err
 		}
 	}
 	return nil
 }
 
+// repoWithToken rewrites a git repository URL to embed the configured access
+// token as basic-auth credentials. When no access token is set it returns the
+// URL unchanged. Extracted from Create() so the rewrite can be unit-tested
+// without shelling out to a real git binary.
+func (vcs *vcsCmd) repoWithToken(repo string) (string, error) {
+	if len(vcs.accessToken) == 0 {
+		return repo, nil
+	}
+	repoURL, err := url.Parse(repo)
+	if err != nil {
+		return "", fmt.Errorf("could not parse repo URL for authenticated clone: %w", err)
+	}
+	repoURL.User = url.UserPassword("x-access-token", vcs.accessToken)
+	return repoURL.String(), nil
+}
+
 func (vcs *vcsCmd) ResetBranchToCommit(dir string, commit string) error {
 	for _, cmd := range vcs.resetToCommitCmd {
 		if err := vcs.run(dir, cmd, "commit", commit); err != nil {

src/jetstream/plugins/cfapppush/vcs_test.go

@@ -0,0 +1,110 @@
+package cfapppush
+
+import (
+	"strings"
+	"testing"
+)
+
+func TestRepoWithToken_NoTokenReturnsURLUnchanged(t *testing.T) {
+	vcs := &vcsCmd{accessToken: ""}
+
+	out, err := vcs.repoWithToken("https://github.com/org/repo.git")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if out != "https://github.com/org/repo.git" {
+		t.Errorf("expected URL to be unchanged when no token is set, got %q", out)
+	}
+}
+
+func TestRepoWithToken_EmbedsTokenAsBasicAuth(t *testing.T) {
+	vcs := &vcsCmd{accessToken: "ghp_testtoken123"}
+
+	out, err := vcs.repoWithToken("https://github.example.com/org/repo.git")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if !strings.Contains(out, "x-access-token:ghp_testtoken123@github.example.com") {
+		t.Errorf("expected URL to contain x-access-token basic-auth, got %q", out)
+	}
+	if !strings.HasPrefix(out, "https://") {
+		t.Errorf("expected URL to preserve https scheme, got %q", out)
+	}
+	if !strings.HasSuffix(out, "/org/repo.git") {
+		t.Errorf("expected URL to preserve path, got %q", out)
+	}
+}
+
+func TestRepoWithToken_InvalidURLReturnsError(t *testing.T) {
+	vcs := &vcsCmd{accessToken: "ghp_testtoken123"}
+
+	// net/url accepts a lot — use a control character to force a parse error.
+	_, err := vcs.repoWithToken("https://\x7fexample.com/repo.git")
+	if err == nil {
+		t.Fatal("expected error on invalid URL, got nil")
+	}
+	if !strings.Contains(err.Error(), "could not parse repo URL") {
+		t.Errorf("expected error to mention 'could not parse repo URL', got %v", err)
+	}
+}
+
+func TestRepoWithToken_SpecialCharactersInTokenEscaped(t *testing.T) {
+	// A token containing URL-special chars (@, :, /, etc.) must be
+	// percent-encoded so downstream git clone doesn't misparse it.
+	vcs := &vcsCmd{accessToken: "abc@def:ghi/jkl"}
+
+	out, err := vcs.repoWithToken("https://github.example.com/org/repo.git")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	// Raw "@" in the token would end the userinfo section prematurely and
+	// point git at the wrong host. Percent-encoding prevents that.
+	if strings.Contains(out, "abc@def") {
+		t.Errorf("expected token special-chars to be percent-encoded, got %q", out)
+	}
+	// URL-encoded "@" is %40
+	if !strings.Contains(out, "abc%40def") {
+		t.Errorf("expected token to be percent-encoded, got %q", out)
+	}
+}
+
+func TestGetVCS_ReturnsFreshCopyNotPrototype(t *testing.T) {
+	// GetVCS must return a copy so callers can't mutate the package-level
+	// prototype and poison subsequent unrelated git operations.
+	a := GetVCS(withAccessToken("token-a"))
+	b := GetVCS() // no options
+
+	if b.accessToken != "" {
+		t.Errorf("expected b.accessToken to be empty (no options applied), got %q", b.accessToken)
+	}
+	if a.accessToken != "token-a" {
+		t.Errorf("expected a.accessToken to be 'token-a', got %q", a.accessToken)
+	}
+	// Mutating a must not affect b
+	a.accessToken = "mutated"
+	if b.accessToken == "mutated" {
+		t.Error("mutation on one GetVCS() result leaked into another call")
+	}
+	// Prototype must also be untouched
+	if vcsGit.accessToken != "" {
+		t.Errorf("package-level vcsGit prototype was mutated; accessToken=%q", vcsGit.accessToken)
+	}
+}
+
+func TestGetVCS_WithAccessTokenOption(t *testing.T) {
+	vcs := GetVCS(withAccessToken("token-xyz"))
+
+	if vcs.accessToken != "token-xyz" {
+		t.Errorf("expected accessToken to be 'token-xyz', got %q", vcs.accessToken)
+	}
+	// Other fields should still be populated from the prototype
+	if vcs.name != "Git" {
+		t.Errorf("expected name to be 'Git', got %q", vcs.name)
+	}
+	if vcs.cmd != "git" {
+		t.Errorf("expected cmd to be 'git', got %q", vcs.cmd)
+	}
+	if len(vcs.createCmd) == 0 {
+		t.Error("expected createCmd to be populated from prototype, got empty slice")
+	}
+}