deps: upgrade google.golang.org/grpc to v1.79.3

[pedrogaudencio]

• upgrade grpc from v1.75.0 to v1.79.3 to fix GHSA-p77j-4mvh-x3m3
• harden Actions runner ConnectRPC method parsing against malformed paths
• add regression coverage for missing leading slash procedure paths

Fixes gRPC-Go has an authorization bypass via missing leading slash in :path

AI Disclosure

Co-authored with: Fable 5

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	golang/​golang.org/​x/​oauth2@​v0.30.0 ⏵ v0.34.0	+1
	golang/​google.golang.org/​grpc@​v1.75.0 ⏵ v1.79.3	+1	+60
	golang/​google.golang.org/​protobuf@​v1.36.8 ⏵ v1.36.10	+1

View full report

[Copilot]

Pull request overview

This PR upgrades google.golang.org/grpc (and related x/* dependencies) to address GHSA-p77j-4mvh-x3m3, and hardens Actions runner request handling by rejecting malformed ConnectRPC procedure paths (notably those missing the required leading /) to prevent unintended auth bypass behavior.

Changes:

• Upgrade google.golang.org/grpc from v1.75.0 to v1.79.3 (plus aligned dependency bumps in go.mod/go.sum).
• Refactor runner interceptor method-name parsing into a stricter helper that requires a leading slash and a valid .../<Method> shape.
• Add regression tests covering malformed procedures, including the missing-leading-slash case.

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.

File	Description
routers/api/actions/runner/interceptor.go	Hardens procedure parsing used to gate unauthenticated Register and update runner activity timestamps.
routers/api/actions/runner/interceptor_test.go	Adds table-driven regression coverage for valid and malformed procedure strings.
go.mod	Bumps gRPC and related module versions to incorporate the security fix.
go.sum	Updates checksums corresponding to the module version upgrades.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[pedrogaudencio]

/review

---

AI review started.

[github-actions]

Advanced AI Review

• Type: Agentic (crush)
• Model: glm-5.2

Click to expand review

I'll investigate the changes thoroughly. Let me start by examining the relevant files and how the modified function is used.Let me verify the dependency changes are consistent (go.sum) and check for any gRPC API usage that could break.go.sum is in sync and the gRPC API surface used is minimal/stable. Let me run the new test and verify compilation.Let me set a writable module cache to run the test.Test passes and go.sum is fully consistent. Let me check whether there are other places in the codebase that parse procedure paths the same (now-hardened) way, which might also need the fix for consistency.Let me check the copyright header convention in this directory and the existing test patterns for consistency.## Review Summary

The core change is sound: the gRPC dependency bump is consistent (go.sum is fully in sync, old versions removed), the API surface used in-tree is tiny and stable (codes/status only — routers/api/actions/runner/runner.go and interceptor.go), the new regression test passes, and no other code path uses the old vulnerable strings.Split(procedure, "/") pattern (confirmed the ping package has no interceptor and the only other .Spec().Procedure access is the one already fixed). The security fix correctly closes the auth bypass: a procedure lacking a leading slash now returns "" instead of "Register", so it no longer short-circuits authentication at interceptor.go:31.

A few minor items below.

---

1. ⚪️ The getMethodName wrapper is now a redundant one-line indirection

• Addressed
• Dismissed

routers/api/actions/runner/interceptor.go:63-65:

func getMethodName(req connect.AnyRequest) string {
	return getMethodNameFromProcedure(req.Spec().Procedure)
}

This wrapper now contains zero logic and is called from exactly one place (interceptor.go:30). It is also the one function in this change that has no test coverage — the regression suite only exercises getMethodNameFromProcedure. Inlining the call removes an untested layer while keeping the tested pure helper:

// interceptor.go:30
methodName := getMethodNameFromProcedure(request.Spec().Procedure)

…and getMethodName can be deleted. If you prefer to keep the request-aware signature for readability, that's defensible, but then consider adding a trivial case to the table that asserts the wrapper still delegates correctly (otherwise the Spec().Procedure wiring is untested).

---

2. ⚪️ The parser is lenient about an empty service segment

• Addressed
• Dismissed

routers/api/actions/runner/interceptor.go:67-77:

func getMethodNameFromProcedure(procedure string) string {
	if !strings.HasPrefix(procedure, "/") {
		return ""
	}

	lastSlash := strings.LastIndex(procedure, "/")
	if lastSlash <= 0 || lastSlash == len(procedure)-1 {
		return ""
	}
	return procedure[lastSlash+1:]
}

This correctly rejects the advisory's vector (no leading slash) and trailing-slash forms, but "/" + "/" + "Register" still resolves to "Register" — i.e. getMethodNameFromProcedure("//Register") returns "Register". I don't think this is exploitable here (gRPC/Connect routing matches on /<service>/<method>, so an empty service name would be rejected as UNIMPLEMENTED before reaching this interceptor), so I'm rating it low confidence. But since the stated intent is "harden … against malformed paths," if you want the helper to be a strict validator rather than a "last segment after the final slash" extractor, a more canonical check would be:

func getMethodNameFromProcedure(procedure string) string {
	// gRPC procedure names are always /<package.Service>/<Method>
	if !strings.HasPrefix(procedure, "/") {
		return ""
	}
	rest := procedure[1:]
	slash := strings.LastIndexByte(rest, "/")
	if slash <= 0 || slash == len(rest)-1 {
		return ""
	}
	return rest[slash+1:]
}

Note this would make the test case "valid prefixed procedure" ("/api/actions/runner.v1.RunnerService/UpdateTask") return "" instead of "UpdateTask". If that prefixed form can genuinely appear in Spec().Procedure, keep the current LastIndex behavior; if not (the HTTP prefix is stripped by the router, so Procedure should always be bare /runner.v1.RunnerService/...), that test row is asserting against a shape that cannot occur in practice and is worth reconsidering either way.

---

Verification note

I could only run the new unit test (it passed); the broader dependency jump is wide — golang.org/x/crypto v0.41.0 → v0.46.0, x/net v0.43 → v0.48, etc., across five x/* modules plus grpc — so make sure CI runs the full make test-backend / test-sqlite suite and not just this one test, since x/crypto is used in security-sensitive paths (SSH, token hashing, bcrypt) elsewhere in the tree.

---

Review generated using glm-5.2 via Z.AI. Comment /review to re-run.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

[pedrogaudencio]

@taoeffect approved! ✅

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.