Summary
civo volume ls can attribute a volume to the wrong instance and the wrong network. The
lookup loops assign to a variable declared outside the loop and break on a match, but there is no
"found" flag — so when no element matches, the variable still holds the last item in the list
and that value is printed as if it were correct.
cmd/volume/volume_list.go:
var instance civogo.Instance
for _, instance = range instances { // assigns to the OUTER variable
if instance.ID == volume.InstanceID {
break
}
}
ow.AppendDataWithLabel("instance_id", instance.Hostname, "Instance") // last instance if no match
Same shape a few lines above, for the Network column:
var network civogo.Network
for _, network = range networks {
if network.ID == volume.NetworkID {
break
}
}
ow.AppendDataWithLabel("network_id", network.Label, "Network") // last network if no match
Contrast the Cluster lookup immediately between them, which is written correctly — it uses a
separate loop variable and a nil check:
var cluster *civogo.KubernetesCluster
for _, c := range clusters.Items {
if c.ID == volume.ClusterID { cluster = &c; break }
}
if cluster != nil { ... } else { ... }
Observed
In lon1, six volumes attached to instance devbox were listed against a different instance
entirely. Before creating any new instances:
| andy-mayastor-enc-kwi2n | ... | devbox | 50 GB | sdb | attached |
After creating an unrelated instance (which changed what ListAllInstances returned last):
| andy-mayastor-enc-kwi2n | ... | repro-gluster-01 | 50 GB | sdb | attached |
Nothing about those volumes changed. They were still attached to devbox. The displayed instance
followed whatever happened to be last in the instance list — which is why the wrong name appeared
only after an unrelated instance was created.
Root cause of the mismatch in that specific case: those volumes belong to a Kubernetes cluster, so
volume.InstanceID refers to a cluster node that is not in ListAllInstances(). The cluster != nil branch is meant to handle that, but when the cluster lookup does not resolve the code falls
into the plain-instance branch, finds no match, and prints the last instance.
Impact
Silently wrong output, not an error — the operator has no signal that the value is unreliable.
Anyone using volume ls to work out what a volume is attached to before detaching or deleting it
can act on the wrong instance. That is a genuinely dangerous read for a destructive follow-up.
The Network column has the same defect by inspection; I observed the Instance column directly and
did not specifically trigger the network case.
Suggested fix
Use a scoped loop variable and an explicit found flag (or slices.IndexFunc) for both lookups, and
print something unambiguous when there is no match — the raw ID, or empty — rather than a
plausible-looking wrong name:
var hostname string
for _, inst := range instances {
if inst.ID == volume.InstanceID {
hostname = inst.Hostname
break
}
}
if hostname == "" {
hostname = volume.InstanceID // or "" — but never another instance's name
}
ow.AppendDataWithLabel("instance_id", hostname, "Instance")
Worth grepping the rest of the CLI for for _, x = range (assignment rather than :=), since the
same pattern will silently misreport anywhere it appears.
Note
The API has the same defect class in a more dangerous place — civo/api-go#276 ("Firewall-rule ops
accept a label as firewallID; no not-found guard defaults to the last firewall -> rule lands on the
wrong firewall"). Same missing not-found guard, same fall-through to the last element. Might be
worth a sweep for the pattern across both codebases.
Found while reproducing a volume-resize bug against lon1.
Summary
civo volume lscan attribute a volume to the wrong instance and the wrong network. Thelookup loops assign to a variable declared outside the loop and
breakon a match, but there is no"found" flag — so when no element matches, the variable still holds the last item in the list
and that value is printed as if it were correct.
cmd/volume/volume_list.go:Same shape a few lines above, for the Network column:
Contrast the Cluster lookup immediately between them, which is written correctly — it uses a
separate loop variable and a nil check:
Observed
In
lon1, six volumes attached to instancedevboxwere listed against a different instanceentirely. Before creating any new instances:
After creating an unrelated instance (which changed what
ListAllInstancesreturned last):Nothing about those volumes changed. They were still attached to
devbox. The displayed instancefollowed whatever happened to be last in the instance list — which is why the wrong name appeared
only after an unrelated instance was created.
Root cause of the mismatch in that specific case: those volumes belong to a Kubernetes cluster, so
volume.InstanceIDrefers to a cluster node that is not inListAllInstances(). Thecluster != nilbranch is meant to handle that, but when the cluster lookup does not resolve the code fallsinto the plain-instance branch, finds no match, and prints the last instance.
Impact
Silently wrong output, not an error — the operator has no signal that the value is unreliable.
Anyone using
volume lsto work out what a volume is attached to before detaching or deleting itcan act on the wrong instance. That is a genuinely dangerous read for a destructive follow-up.
The Network column has the same defect by inspection; I observed the Instance column directly and
did not specifically trigger the network case.
Suggested fix
Use a scoped loop variable and an explicit found flag (or
slices.IndexFunc) for both lookups, andprint something unambiguous when there is no match — the raw ID, or empty — rather than a
plausible-looking wrong name:
Worth grepping the rest of the CLI for
for _, x = range(assignment rather than:=), since thesame pattern will silently misreport anywhere it appears.
Note
The API has the same defect class in a more dangerous place — civo/api-go#276 ("Firewall-rule ops
accept a label as firewallID; no not-found guard defaults to the last firewall -> rule lands on the
wrong firewall"). Same missing not-found guard, same fall-through to the last element. Might be
worth a sweep for the pattern across both codebases.
Found while reproducing a volume-resize bug against
lon1.