Skip to content

chore(notebook-controller): fix of some of the linter errors #780

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jstourac wants to merge 1 commit into
base: main
Choose a base branch
from
+47 −32
Open

chore(notebook-controller): fix of some of the linter errors #780

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions components/notebook-controller/controllers/culling_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,11 @@ func getNotebookApiKernels(nm, ns string, log logr.Logger) []KernelStatus {

var kernels []KernelStatus

defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(resp.Body).Decode(&kernels)
if err != nil {
Comment on lines +285 to 291
Copy link

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

fd -t f "culling_controller.go" --exec wc -l {} \;

Repository: opendatahub-io/kubeflow

Length of output: 137

🏁 Script executed:

fd -t f "culling_controller.go" --exec cat -n {} \; | sed -n '280,320p'

Repository: opendatahub-io/kubeflow

Length of output: 1310

🏁 Script executed:

fd -t f "culling_controller.go" --exec grep -n "json.NewDecoder" {} \;

Repository: opendatahub-io/kubeflow

Length of output: 180

🏁 Script executed:

fd -t f "culling_controller.go" --exec head -n 30 {} \; | grep -E "^import|^import \(|\"io\"|\"encoding/json"|\"net/http"

Repository: opendatahub-io/kubeflow

Length of output: 135

🏁 Script executed:

fd -t f "culling_controller.go" --exec cat -n {} \; | sed -n '1,50p'

Repository: opendatahub-io/kubeflow

Length of output: 2079

Critical: bound Notebook API body decoding to prevent memory-exhaustion DoS (CWE-400).

Lines 290 and 314 decode HTTP responses without size limits. A compromised or malicious notebook endpoint can exhaust controller memory by streaming oversized JSON, destabilizing reconciliation.

Fix 
 import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"strconv"
 	"time"
@@
 const DEFAULT_DEV = "false"
+const MAX_NOTEBOOK_API_BODY_BYTES int64 = 1 << 20 // 1 MiB
@@
-	err := json.NewDecoder(resp.Body).Decode(&kernels)
+	err := json.NewDecoder(io.LimitReader(resp.Body, MAX_NOTEBOOK_API_BODY_BYTES)).Decode(&kernels)
@@
-	err := json.NewDecoder(resp.Body).Decode(&terminals)
+	err := json.NewDecoder(io.LimitReader(resp.Body, MAX_NOTEBOOK_API_BODY_BYTES)).Decode(&terminals)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/notebook-controller/controllers/culling_controller.go` around
lines 285 - 291, The code currently decodes resp.Body directly via
json.NewDecoder(resp.Body).Decode(&kernels) which allows an attacker to stream
arbitrarily large JSON and exhaust memory; fix by wrapping the response body
with a size limit (e.g. io.LimitReader(resp.Body, maxBodySize)) or read into an
io.LimitedReader before creating the decoder, define a reasonable constant like
maxNotebookResponseBytes, then replace json.NewDecoder(resp.Body) with
json.NewDecoder(io.LimitReader(resp.Body, maxNotebookResponseBytes)) and handle
the case where the limit is hit (return an explicit error/log) before closing
resp.Body; ensure the same pattern is applied anywhere resp.Body is decoded
(references: resp.Body, json.NewDecoder, kernels).

log.Error(err, "Error parsing JSON response for Notebook API Kernels.")
Expand All @@ -302,7 +306,11 @@ func getNotebookApiTerminals(nm, ns string, log logr.Logger) []TerminalStatus {

var terminals []TerminalStatus

defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(resp.Body).Decode(&terminals)
if err != nil {
log.Error(err, "Error parsing JSON response for Notebook API terminals.")
Expand Down Expand Up @@ -370,7 +378,7 @@ func compareAnnotationTimeToResource(meta *metav1.ObjectMeta, resourceTime strin

func updateTimestampFromKernelsActivity(meta *metav1.ObjectMeta, kernels []KernelStatus, log logr.Logger) bool {

if kernels == nil || len(kernels) == 0 {
if len(kernels) == 0 {
log.Info("Notebook has no kernels. Will not update last-activity")
return false
}
Expand All @@ -391,7 +399,7 @@ func updateTimestampFromKernelsActivity(meta *metav1.ObjectMeta, kernels []Kerne
}

t := getNotebookRecentTime(arr, "api/kernels", log)
log.Info(fmt.Sprintf("Comparing api/kernels last_activity time to current notebook annotation time"))
log.Info("Comparing api/kernels last_activity time to current notebook annotation time")
if t == "" || !compareAnnotationTimeToResource(meta, t, log) {
return false
}
Expand All @@ -406,7 +414,7 @@ func updateTimestampFromTerminalsActivity(meta *metav1.ObjectMeta, terminals []T
// check this timestamp against the current annotation timestamp to ensure we are not
// going backwards in time.

if terminals == nil || len(terminals) == 0 {
if len(terminals) == 0 {
log.Info("Notebook has no terminals. Will not update last-activity")
return false
}
Expand All @@ -417,7 +425,7 @@ func updateTimestampFromTerminalsActivity(meta *metav1.ObjectMeta, terminals []T
}

t := getNotebookRecentTime(arr, "api/terminals", log)
log.Info(fmt.Sprintf("Comparing api/terminals last_activity time to current notebook annotation time"))
log.Info("Comparing api/terminals last_activity time to current notebook annotation time")
if t == "" || !compareAnnotationTimeToResource(meta, t, log) {
return false
}
Expand Down
8 changes: 6 additions & 2 deletions components/notebook-controller/controllers/culling_controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,13 @@ func TestNotebookIsIdle(t *testing.T) {
for _, c := range testCases {
t.Run(c.testName, func(t *testing.T) {
for envVar, val := range c.env {
os.Setenv(envVar, val)
if err := os.Setenv(envVar, val); err != nil {
t.Fatalf("Failed to set env var %s: %v", envVar, err)
}
}
Comment on lines +254 to +257
Copy link

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Check the file and get context around lines 254-257
wc -l components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 140

🏁 Script executed:

# Read the file around the specified lines with context
sed -n '240,270p' components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 792

🏁 Script executed:

# Get a broader view of the test structure to understand if it's using subtests
sed -n '200,280p' components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 1819

🏁 Script executed:

# Check Go version requirement
fd -e "go.mod" | head -1 | xargs cat | grep "^go "

Repository: opendatahub-io/kubeflow

Length of output: 49

🏁 Script executed:

# Verify the actual current code is as expected
sed -n '254,257p' components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 187

🏁 Script executed:

# Check the test function signature to understand the testing context better
sed -n '200,260p' components/notebook-controller/controllers/culling_controller_test.go | head -70

Repository: opendatahub-io/kubeflow

Length of output: 1651

🏁 Script executed:

# Look for the test function name and start
rg "func Test.*notebookIsIdle" -A 5 components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 49

🏁 Script executed:

# Search for the test function that contains the code around line 254
rg "func Test" components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 237

🏁 Script executed:

# Get the full context of the issue - from the test function declaration onwards
sed -n '170,280p' components/notebook-controller/controllers/culling_controller_test.go

Repository: opendatahub-io/kubeflow

Length of output: 2498

Use t.Setenv to avoid cross-test environment leakage.

The os.Setenv calls inside t.Run subtests mutate global process state without cleanup. Subtests with empty env maps will inherit environment variables from prior test cases, creating order-dependent test behavior. Replace with t.Setenv, which automatically restores the environment when each subtest completes.

Proposed fix 
-			for envVar, val := range c.env {
-				if err := os.Setenv(envVar, val); err != nil {
-					t.Fatalf("Failed to set env var %s: %v", envVar, err)
-				}
-			}
+			for envVar, val := range c.env {
+				t.Setenv(envVar, val)
+			}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if err := os.Setenv(envVar, val); err != nil {
t.Fatalf("Failed to set env var %s: %v", envVar, err)
}
}
for envVar, val := range c.env {
t.Setenv(envVar, val)
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/notebook-controller/controllers/culling_controller_test.go` around
lines 254 - 257, The test mutates global env with os.Setenv inside the t.Run
subtests (loop setting envVar/val) causing cross-test leakage; replace os.Setenv
calls with t.Setenv(envVar, val) inside the subtest so the testing framework
automatically restores the environment after each subtest (update the loop that
currently calls os.Setenv(err := os.Setenv(envVar, val)) to use t.Setenv(envVar,
val) and remove manual cleanup).

if err := initGlobalVars(); err != nil {
t.Fatalf("Failed to initialize global vars: %v", err)
}
initGlobalVars()
if notebookIsIdle(c.meta, TestLogger) != c.result {
t.Errorf("ENV VAR: %+v\n", c.env)
t.Errorf("Wrong result for case object: %+v\n", c.meta)
Expand Down
23 changes: 11 additions & 12 deletions components/notebook-controller/controllers/notebook_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,7 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c

// TODO(yanniszark): Can we avoid reconciling Events and Notebook in the same queue?
event := &corev1.Event{}
var getEventErr error
getEventErr = r.Get(ctx, req.NamespacedName, event)
getEventErr := r.Get(ctx, req.NamespacedName, event)
if getEventErr == nil {
log.Info("Found event for Notebook. Re-emitting...")

Expand Down Expand Up @@ -185,11 +184,11 @@ func (r *NotebookReconciler) Reconcile(ctx context.Context, req ctrl.Request) (c
return ctrl.Result{}, err
}

// Copy the pod template labels, but reconcilation is not required
// exclusively based on ths pod template labels
// Copy the pod template labels, but reconciliation is not required
// exclusively based on the pod template labels
if ss.Spec.Replicas != nil && foundStateful.Spec.Replicas != nil && *ss.Spec.Replicas != *foundStateful.Spec.Replicas {
if !reflect.DeepEqual(foundStateful.Spec.Template.ObjectMeta.Labels, ss.Spec.Template.ObjectMeta.Labels) {
foundStateful.Spec.Template.ObjectMeta.Labels = ss.Spec.Template.ObjectMeta.Labels
if !reflect.DeepEqual(foundStateful.Spec.Template.Labels, ss.Spec.Template.Labels) {
foundStateful.Spec.Template.Labels = ss.Spec.Template.Labels
}
Comment on lines +190 to 192
Copy link

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# 1) Inspect CopyStatefulSetFields behavior for ObjectMeta/Template label reconciliation
rg -nP --type=go 'func\s+CopyStatefulSetFields\s*\(' -C20

# 2) Show all StatefulSet label assignments in notebook controller
rg -nP --type=go 'foundStateful\.(Labels|Spec\.Template\.Labels)|ss\.(Labels|Spec\.Template\.Labels)' components/notebook-controller/controllers/notebook_controller.go -C3

# 3) Re-check test expectations around StatefulSet object labels
rg -nP 'Expect\(sts\.GetLabels\(\)\)\.To\(Equal\(notebook\.GetLabels\(\)\)\)' components/notebook-controller/controllers/notebook_controller_bdd_test.go -C3

Repository: opendatahub-io/kubeflow

Length of output: 3997

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Get the full reconciliation logic around lines 189-192
sed -n '170,210p' components/notebook-controller/controllers/notebook_controller.go

# Check if CopyStatefulSetFields is called before or after the replica check
rg -nP 'CopyStatefulSetFields' components/notebook-controller/controllers/notebook_controller.go -B5 -A5

# Verify what triggers the entire reconciliation block
sed -n '180,200p' components/notebook-controller/controllers/notebook_controller.go

Repository: opendatahub-io/kubeflow

Length of output: 2991

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for any other template label assignments/reconciliation in notebook controller
rg -nP 'Spec\.Template\.Labels\s*=' components/notebook-controller/controllers/notebook_controller.go

# Verify CopyStatefulSetFields doesn't handle template labels
sed -n '107,140p' components/common/reconcilehelper/util.go

Repository: opendatahub-io/kubeflow

Length of output: 933

Template labels lag behind ObjectMeta labels on label-only updates.

Template label reconciliation is incorrectly gated by the replica check (lines 189–192). When a Notebook's labels change without replica scaling, CopyStatefulSetFields syncs ObjectMeta labels unconditionally, but Template.Labels remain stale. This causes pod template label drift, breaking pod scheduling selectors, network policies, and label-based identity.

Move the template label sync outside the replica condition:

Fix 
-	if ss.Spec.Replicas != nil && foundStateful.Spec.Replicas != nil && *ss.Spec.Replicas != *foundStateful.Spec.Replicas {
-		if !reflect.DeepEqual(foundStateful.Spec.Template.Labels, ss.Spec.Template.Labels) {
-			foundStateful.Spec.Template.Labels = ss.Spec.Template.Labels
-		}
-	}
+	if !reflect.DeepEqual(foundStateful.Spec.Template.Labels, ss.Spec.Template.Labels) {
+		foundStateful.Spec.Template.Labels = ss.Spec.Template.Labels
+	}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/notebook-controller/controllers/notebook_controller.go` around
lines 190 - 192, The template labels on the StatefulSet are only being updated
inside the replica-change conditional (the block comparing replicas), so
label-only updates leave foundStateful.Spec.Template.Labels stale; move the
block that compares and assigns Template.Labels (the check using
reflect.DeepEqual on foundStateful.Spec.Template.Labels vs
ss.Spec.Template.Labels and the assignment foundStateful.Spec.Template.Labels =
ss.Spec.Template.Labels) out of the replica-reconciliation branch so it runs
unconditionally whenever CopyStatefulSetFields has synced ObjectMeta labels;
ensure this runs every reconciliation pass (e.g., after CopyStatefulSetFields)
so ss/ foundStateful template labels are always kept in sync.

}

Expand Down Expand Up @@ -477,14 +476,14 @@ func generateStatefulSet(instance *v1beta1.Notebook, isGenerateName bool) *appsv
}

// copy all of the Notebook labels to the pod including poddefault related labels
l := &ss.Spec.Template.ObjectMeta.Labels
for k, v := range instance.ObjectMeta.Labels {
l := &ss.Spec.Template.Labels
for k, v := range instance.Labels {
(*l)[k] = v
}

// copy all of the Notebook annotations to the pod.
a := &ss.Spec.Template.ObjectMeta.Annotations
for k, v := range instance.ObjectMeta.Annotations {
a := &ss.Spec.Template.Annotations
for k, v := range instance.Annotations {
if !strings.Contains(k, "kubectl") && !strings.Contains(k, "notebook") {
(*a)[k] = v
}
Expand Down Expand Up @@ -563,7 +562,7 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu

// unpack annotations from Notebook resource
annotations := make(map[string]string)
for k, v := range instance.ObjectMeta.Annotations {
for k, v := range instance.Annotations {
annotations[k] = v
}

Expand All @@ -589,7 +588,7 @@ func generateVirtualService(instance *v1beta1.Notebook) (*unstructured.Unstructu
istioHost = "*"
}
if err := unstructured.SetNestedStringSlice(vsvc.Object, []string{istioHost}, "spec", "hosts"); err != nil {
return nil, fmt.Errorf("Set .spec.hosts error: %v", err)
return nil, fmt.Errorf("set .spec.hosts error: %v", err)

}

Expand Down
28 changes: 16 additions & 12 deletions components/notebook-controller/pkg/culler/culler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,11 +133,7 @@ func SetStopAnnotation(meta *metav1.ObjectMeta, m *metrics.Metrics) {
m.NotebookCullingTimestamp.WithLabelValues(meta.Namespace, meta.Name).Set(float64(t.Unix()))
}

if meta.GetAnnotations() != nil {
if _, ok := meta.GetAnnotations()["notebooks.kubeflow.org/last_activity"]; ok {
delete(meta.GetAnnotations(), "notebooks.kubeflow.org/last_activity")
}
}
delete(meta.GetAnnotations(), "notebooks.kubeflow.org/last_activity")
Copy link

@coderabbitai coderabbitai bot Mar 18, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Wrong annotation key is deleted; stale last-activity remains.

The code deletes "notebooks.kubeflow.org/last_activity" (underscore), but the active key is LAST_ACTIVITY_ANNOTATION (.../last-activity, hyphen). This leaves the intended annotation untouched.

Proposed fix 
-	delete(meta.GetAnnotations(), "notebooks.kubeflow.org/last_activity")
+	delete(meta.GetAnnotations(), LAST_ACTIVITY_ANNOTATION)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
delete(meta.GetAnnotations(), "notebooks.kubeflow.org/last_activity")
delete(meta.GetAnnotations(), LAST_ACTIVITY_ANNOTATION)
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/notebook-controller/pkg/culler/culler.go` at line 136, The code is
deleting the wrong annotation key (uses "notebooks.kubeflow.org/last_activity"
with an underscore), leaving the actual LAST_ACTIVITY_ANNOTATION
(".../last-activity") intact; update the deletion to remove the correct
annotation by calling delete(meta.GetAnnotations(), LAST_ACTIVITY_ANNOTATION) or
using the hyphenated string constant, ensuring the old underscore key is no
longer relied upon.

Copy link
Member

@harshad16 harshad16 Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jstourac lets check this , please is the annotation really been wrong all this while ?

Copy link

@coderabbitai coderabbitai bot Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!

Copy link
Member Author

@jstourac jstourac Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it does seem like that - as I mentioned below, I didn't want to do any other changes here except the linter fixes. I have a followup branch (wip PR) with changes that should address all the comments from the bot here including this one https://github.com/opendatahub-io/kubeflow/pull/781/changes#diff-bfc792d035f63eea8527be116419aca884baf8d8bc2c3e758408e4e7e621af7eR137 . Will open a separate smaller PRs later on so that it's easier to review and faster to merge 🙂

}

func StopAnnotationIsSet(meta metav1.ObjectMeta) bool {
Expand Down Expand Up @@ -190,7 +186,11 @@ func getNotebookApiKernels(nm, ns string) []KernelStatus {

var kernels []KernelStatus

defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(resp.Body).Decode(&kernels)
if err != nil {
Comment on lines +189 to 195
Copy link

@coderabbitai coderabbitai bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

cat -n components/notebook-controller/pkg/culler/culler.go | sed -n '180,230p'

Repository: opendatahub-io/kubeflow

Length of output: 1748

🏁 Script executed:

cat -n components/notebook-controller/pkg/culler/culler.go | sed -n '1,180p' | tail -100

Repository: opendatahub-io/kubeflow

Length of output: 3611

🏁 Script executed:

head -50 components/notebook-controller/pkg/culler/culler.go

Repository: opendatahub-io/kubeflow

Length of output: 1728

🏁 Script executed:

rg -n "json\.(NewDecoder|Unmarshal)" components/notebook-controller/pkg/culler/culler.go

Repository: opendatahub-io/kubeflow

Length of output: 180

CWE-400: Unbounded JSON decode on HTTP response bodies allows memory exhaustion (lines 194, 218).

Severity: Critical. Notebook API response body is decoded without size limits; malicious or oversized response exhausts controller memory and disrupts reconciliation.

Proposed fix 
 import (
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 	"strconv"
 	"time"
@@
 const DEFAULT_DEV = "false"
+const MAX_NOTEBOOK_API_BODY_BYTES int64 = 1 << 20 // 1 MiB
@@
-	err := json.NewDecoder(resp.Body).Decode(&kernels)
+	err := json.NewDecoder(io.LimitReader(resp.Body, MAX_NOTEBOOK_API_BODY_BYTES)).Decode(&kernels)
@@
-	err := json.NewDecoder(resp.Body).Decode(&terminals)
+	err := json.NewDecoder(io.LimitReader(resp.Body, MAX_NOTEBOOK_API_BODY_BYTES)).Decode(&terminals)

Per Go security guidelines: Use io.LimitReader for HTTP response bodies to prevent memory exhaustion.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(resp.Body).Decode(&kernels)
if err != nil {
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(io.LimitReader(resp.Body, MAX_NOTEBOOK_API_BODY_BYTES)).Decode(&kernels)
if err != nil {
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@components/notebook-controller/pkg/culler/culler.go` around lines 189 - 195,
The code decodes HTTP response bodies with json.NewDecoder(resp.Body) into
kernels without bounds; replace direct use of resp.Body with an io.LimitReader
to prevent memory exhaustion (e.g., lr := io.LimitReader(resp.Body,
maxJSONSize)) and pass lr to json.NewDecoder instead; add a package-level
constant like maxJSONSize (e.g., 10<<20 for 10MB) and ensure io is imported;
apply the same change for the other json.Decode usage around the later decode
(the second json.NewDecoder call) and keep the existing resp.Body.Close() defer.

log.Error(err, "Error parsing JSON response for Notebook API Kernels.")
Expand All @@ -210,7 +210,11 @@ func getNotebookApiTerminals(nm, ns string) []TerminalStatus {

var terminals []TerminalStatus

defer resp.Body.Close()
defer func() {
if err := resp.Body.Close(); err != nil {
log.Error(err, "Error closing response body.")
}
}()
err := json.NewDecoder(resp.Body).Decode(&terminals)
if err != nil {
log.Error(err, "Error parsing JSON response for Notebook API terminals.")
Expand Down Expand Up @@ -263,11 +267,11 @@ func getNotebookRecentTime(t []string, api string) string {

// Update LAST_ACTIVITY_ANNOTATION
func UpdateNotebookLastActivityAnnotation(meta *metav1.ObjectMeta) bool {
log := log.WithValues("notebook", getNamespacedNameFromMeta(*meta))
if meta == nil {
log.Info("Metadata is Nil. Can't update Last Activity Annotation.")
return false
}
log := log.WithValues("notebook", getNamespacedNameFromMeta(*meta))

log.Info("Updating the last-activity annotation.")
nm, ns := meta.GetName(), meta.GetNamespace()
Expand Down Expand Up @@ -322,7 +326,7 @@ func compareAnnotationTimeToResource(meta *metav1.ObjectMeta, resourceTime strin
func updateTimestampFromKernelsActivity(meta *metav1.ObjectMeta, kernels []KernelStatus) bool {
log := log.WithValues("notebook", getNamespacedNameFromMeta(*meta))

if kernels == nil || len(kernels) == 0 {
if len(kernels) == 0 {
log.Info("Notebook has no kernels. Will not update last-activity")
return false
}
Expand All @@ -343,7 +347,7 @@ func updateTimestampFromKernelsActivity(meta *metav1.ObjectMeta, kernels []Kerne
}

t := getNotebookRecentTime(arr, "api/kernels")
log.Info(fmt.Sprintf("Comparing api/kernels last_activity time to current notebook annotation time"))
log.Info("Comparing api/kernels last_activity time to current notebook annotation time")
if t == "" || !compareAnnotationTimeToResource(meta, t) {
return false
}
Expand All @@ -359,7 +363,7 @@ func updateTimestampFromTerminalsActivity(meta *metav1.ObjectMeta, terminals []T
// going backwards in time.
log := log.WithValues("notebook", getNamespacedNameFromMeta(*meta))

if terminals == nil || len(terminals) == 0 {
if len(terminals) == 0 {
log.Info("Notebook has no terminals. Will not update last-activity")
return false
}
Expand All @@ -370,7 +374,7 @@ func updateTimestampFromTerminalsActivity(meta *metav1.ObjectMeta, terminals []T
}

t := getNotebookRecentTime(arr, "api/terminals")
log.Info(fmt.Sprintf("Comparing api/terminals last_activity time to current notebook annotation time"))
log.Info("Comparing api/terminals last_activity time to current notebook annotation time")
if t == "" || !compareAnnotationTimeToResource(meta, t) {
return false
}
Expand Down