AI found some issues, fix them

[majst01]

Description

1. Nil-pointer dereference before nil check — pkg/request/authorize.go:43-46

✅

func (a *authorizer) Authorize(ctx context.Context, token *apiv2.Token, req connect.AnyRequest) error {
	var (
		method  = req.Spec().Procedure // deref happens HERE
		subject string
	)
	if req == nil {                    // check happens AFTER — dead check
		return errorutil.Internal("request is nil")
	}

req.Spec() is evaluated before the req == nil guard, so the guard can never fire.
A nil req panics instead of returning the intended Internal error.

Fix: move the nil check above line 43.

2. Inverted guard in admin machine List — pkg/service/admin/machine/machine-service.go:77-85

✅

partition := rq.Partition
if partition != nil {                       // guard inverted; should be == nil
	if len(partitions) > 1 {
		return nil, errorutil.InvalidArgument("no partition specified, but %d partitions available", len(partitions))
	}
	if len(partitions) == 1 {
		partition = &partitions[0].Id       // silently overwrites the user's requested partition
	}
}

The error only triggers when the user did specify a partition, with a message saying
"no partition specified". When the user specifies no partition and there are many available,
it silently lists across all partitions. It also overwrites an explicitly-requested partition
with the single available one.

Fix: the guard should be if partition == nil.

3. Dead / unreachable check — pkg/service/api/tenant/tenant-service.go:120-126

✅

if pointer.SafeDeref(req.Email) == "" && ownTenant.Email != "" {
	req.Email = new(ownTenant.Email)
	if pointer.SafeDeref(req.Email) == "" {   // impossible: == ownTenant.Email which was just checked != ""
		return nil, errorutil.FailedPrecondition("email is required")
	}
}

The inner empty-check is dead code — req.Email was just assigned ownTenant.Email,
which the guard already established is non-empty. Either the guard intent is wrong or
the inner check is misplaced.

Fix: clarify intent; remove the dead branch or fix the guard.

---

Consistency / maintenance concerns

4. Inconsistent error wrapping

✅
pkg/repository/network.go:126-132 uses errorutil.Convert(err), while
pkg/service/admin/tenant/tenant-service.go:76-87 uses errorutil.Internal("...%v", err)
which discards the wrapped error via %v instead of %w. Some handlers wrap with %w,
others with %v — the %v cases lose the underlying error for errors.Is/As.

Fix: normalize error wrapping to %w across handlers.

5. Variable shadowing in network create — pkg/repository/network.go:112 & 141

✅
An outer vrf uint is declared, then a shadowing var vrf uint is redeclared inside the
switch case (line 141). The Vrf: vrf used at line 182 refers to the inner one, while
lines 226-232 mutate the outer one. Confusing and bug-prone.

Fix: collapse the duplicate vrf declarations into one.

Used AI-Tools ✨

• Deepseek V4 Flash used for generation

[codecov]

Codecov Report

❌ Patch coverage is 28.57143% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.95%. Comparing base (bb7c6ce) to head (e670ab0).

Files with missing lines	Patch %	Lines
pkg/repository/network.go	0.00%	3 Missing ⚠️
pkg/request/authorize.go	50.00%	1 Missing ⚠️
pkg/service/admin/tenant/tenant-service.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #299      +/-   ##
==========================================
+ Coverage   79.93%   79.95%   +0.02%     
==========================================
  Files         199      199              
  Lines       14800    14797       -3     
==========================================
+ Hits        11830    11831       +1     
+ Misses       2969     2965       -4     
  Partials        1        1              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[iljarotar]

Shouldn't we still check if email is empty? Maybe move this check one line lower?

[iljarotar]

Also, shouldn't email be a required field?

[majst01]

but req.Email is overwritten in L121 with ownTenant.Email which is checked for not "" in L120

[iljarotar]

Yes, but in case ownTenant.Email == "" it will remain empty and should be checked if we require it. If an email is not required then it's fine like this.