🤖 Code Audit: 17 potential issue(s) found

[asmit25805]

Code Audit Report

All findings are reviewed for confidence before posting.
Please verify each finding before acting on it.

Repository: Tong89/smartNode
Findings: 17 issue(s) found — 🔴 1 critical · 🟠 10 high · 🟡 6 medium

---

1. 🐛 Incomplete statement causing syntax error

Field	Details
Severity	🔴 Critical
Type	Bug
File	backend/api.py
Location	get_resource_timeline function (truncated part)
Confidence	96%

Problem:
The function get_resource_timeline ends with an incomplete line timeline_d inside a conditional block. This results in a syntax error, preventing the Flask application from starting or the endpoint from being reachable, which is a runtime failure in normal use.

Suggested Fix:
Remove the stray timeline_d line and ensure the conditional block correctly appends the event to the appropriate timeline dictionary, e.g.,

if req.selected_relay2 in timeline_data["geo_relays"]:
    timeline_data["geo_relays"][req.selected_relay2]["events"].append(event.copy())

---

2. 🐛 Inconsistent OrbitalElements initialization

Field	Details
Severity	🟠 High
Type	Bug
File	backend/constellation.py
Location	LEO_SATELLITES list
Confidence	95%

Problem:
The OrbitalElements instances in the LEO_SATELLITES list are initialized with different parameters. The first instance is initialized with a TLE, while the others are initialized with Keplerian elements. This inconsistency may lead to incorrect calculations or errors.

Suggested Fix:
Ensure consistent initialization of OrbitalElements instances, either using TLE or Keplerian elements throughout the LEO_SATELLITES list.

---

3. 🐛 Potential IndexError in ground station sampling

Field	Details
Severity	🟠 High
Type	Bug
File	backend/core.py
Location	self.ground_stations = self.rng.sample(self.all_ground_stations, self.ground_stat
Confidence	95%

Problem:
The code uses self.rng.sample to select a random subset of ground stations, but it does not check if the sample size is larger than the population size. If self.ground_station_count is larger than the length of self.all_ground_stations, this will raise an IndexError.

Suggested Fix:
Add a check to ensure that self.ground_station_count is not larger than the length of self.all_ground_stations before calling self.rng.sample.

---

4. 🐛 Uses private _value attribute of Counter instead of public inc()

Field	Details
Severity	🟠 High
Type	Bug
File	backend/metrics.py
Location	update_simulation_metrics
Confidence	96%

Problem:
The function update_simulation_metrics accesses Counter objects via their internal _value attribute (e.g., sim_total_requests_total._value.inc(...)). This relies on undocumented internals; if the Counter implementation changes or the attribute is absent, an AttributeError will be raised, breaking metric updates at runtime.

Suggested Fix:
Replace the private attribute access with the public Counter API: use sim_total_requests_total.inc(delta) and similarly for the other counters.

---

5. 🐛 Incorrect distance argument for GEO‑to‑ground‑station leg

Field	Details
Severity	🟠 High
Type	Bug
File	backend/orbit.py
Location	calculate_multi_hop_relay_rate
Confidence	96%

Problem:
In calculate_multi_hop_relay_rate the call to link_budget_relay uses dist_geo_gs_km=dist1_km, which is the LEO‑to‑GEO distance, instead of the GEO‑to‑ground‑station distance. This causes the relay link budget to be computed with a wrong path length, leading to inaccurate (typically overly optimistic) rate calculations.

Suggested Fix:
Compute the correct GEO‑to‑ground‑station slant range (e.g., using _slant_range_km(geo2_pos, gs)) and pass it as dist_geo_gs_km to link_budget_relay. Example:

    dist_geo_gs_km = _slant_range_km(geo2_pos, {
        "lat": gs["lat"],
        "lon": gs["lon"],
        "alt": gs.get("alt", 0.0),
    })
    res1 = link_budget_relay(dist_leo_geo_km=dist1_km, dist_geo_gs_km=dist_geo_gs_km, data_type=data_type)

---

6. 🐛 Missing time slot conflict check in occupy()

Field	Details
Severity	🟠 High
Type	Bug
File	backend/resources.py
Location	occupy
Confidence	95%

Problem:
The occupy() method directly reserves time slots via reserve_time_slot() without verifying that the requested time interval is free. This can lead to overlapping reservations and inconsistent state, causing resource overallocation and later failures when checking availability.

Suggested Fix:
Before calling reserve_time_slot(), invoke check_time_slot_available() for each resource (satellite, ground station, relays) and abort or raise an exception if the slot is not available.

---

7. 🐛 Incorrect handling of 'data_types' field causing AttributeError

Field	Details
Severity	🟠 High
Type	Bug
File	backend/scenario.py
Location	load_scenario
Confidence	96%

Problem:
The function load_scenario assumes that the data_types entry in the JSON file is a dictionary and iterates over raw.get("data_types", {}).items(). However, the documented Scenario Schema defines data_types as a list of strings. When a conforming scenario file provides a list, calling .items() on a list raises an AttributeError, breaking scenario loading.

Suggested Fix:
Update load_scenario to handle both list and dict representations. If data_types is a list, convert it to a dict with default configurations or simply return the list. Example fix:

raw_data_types = raw.get("data_types", {})
if isinstance(raw_data_types, dict):
    data_types = {}
    for k, v in raw_data_types.items():
        cfg = dict(v)
        if "size_range" in cfg and isinstance(cfg["size_range"], list):
            cfg["size_range"] = tuple(cfg["size_range"])
        if "priority_range" in cfg and isinstance(cfg["priority_range"], list):
            cfg["priority_range"] = tuple(cfg["priority_range"])
        data_types[k] = cfg
elif isinstance(raw_data_types, list):
    # Preserve the list as‑is or map to empty config dicts if needed
    data_types = {key: {} for key in raw_data_types}
else:
    data_types = {}

Adjust the return value accordingly, ensuring compatibility with both schema versions.

---

8. 🐛 NaN values pass numeric validation

Field	Details
Severity	🟠 High
Type	Bug
File	backend/schemas.py
Location	validate_request_submission (data_size and max_delay checks)
Confidence	95%

Problem:
The helper _is_number returns True for float('nan'), and the subsequent checks only verify that the value is > 0 (for data_size) or >= 0 (for max_delay). Since NaN comparisons are always False, NaN values are accepted as valid, allowing invalid payloads that can cause downstream arithmetic errors or undefined behavior.

Suggested Fix:
Add an explicit check for NaN (e.g., using math.isnan) in _is_number or in the validation branches, rejecting NaN values as invalid numbers.

---

9. 🐛 Assumption that resource usage entries are iterable can cause TypeError

Field	Details
Severity	🟠 High
Type	Bug
File	backend/snapshot.py
Location	restore method (lines handling engine._resources.usage)
Confidence	96%

Problem:
The restore() method reconstructs resource usage by iterating over snapshot['resource_usage']['satellites'], 'ground_stations' and 'geo_relays' and converting each value with list(v). If a corrupted or malicious snapshot contains a non‑iterable value (e.g., an int or None), list(v) raises a TypeError, aborting the restore process and leaving the engine in an inconsistent state.

Suggested Fix:
Validate that each value is iterable before calling list(), or copy the structure without conversion. Example:

raw_sat = raw_usage.get("satellites", {})
engine._resources.usage["satellites"] = {
    k: list(v) if isinstance(v, (list, tuple, set)) else []
    for k, v in raw_sat.items()
}

Add similar checks for ground_stations and geo_relays, and report a clear error if the snapshot is malformed.

---

10. 🐛 SQL Injection Vulnerability in _init_schema Method

Field	Details
Severity	🟠 High
Type	Bug
File	backend/store.py
Location	ScenarioStore._init_schema
Confidence	95%

Problem:
The _init_schema method is vulnerable to SQL injection attacks because it uses string formatting to construct SQL queries. An attacker could exploit this vulnerability by injecting malicious SQL code.

Suggested Fix:
Use parameterized queries or prepared statements to prevent SQL injection attacks.

---

11. 🐛 Potential deadlock in event_bus.publish

Field	Details
Severity	🟠 High
Type	Bug
File	backend/stream.py
Location	event_bus.publish
Confidence	95%

Problem:
The event_bus.publish method acquires a lock and then iterates over the subscribers. If a subscriber is too slow and its queue is full, the method will try to remove the oldest item from the queue and retry. However, if the queue is still full after removing the oldest item, the method will append the subscriber to the 'dead' list and try to remove it from the subscribers list. This can lead to a deadlock if another thread is trying to subscribe or unsubscribe a subscriber at the same time.

Suggested Fix:
Consider using a separate lock for the 'dead' list or using a more efficient data structure for the subscribers list.

---

12. 🐛 Prefix check ignores normalized path, breaking versioned public prefixes

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/auth.py
Location	is_public_path
Confidence	95%

Problem:
The function normalizes paths that start with "/api/v1/" to "/api/" for exact‑match checks, but when checking prefix whitelists it still uses the original path. Consequently, requests like "/api/v1/frontend/..." are not recognized as public because the prefix "/frontend" is compared against the unnormalized path, leading to unintended authentication failures.

Suggested Fix:
Apply the same normalization to the prefix check, e.g., replace the final return with return any(normalized.startswith(p) for p in PUBLIC_PREFIXES).

---

13. 🐛 Potential KeyError in GEO_RELAY_SATELLITES

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/constellation.py
Location	GEO_RELAY_SATELLITES list
Confidence	92%

Problem:
The GEO_RELAY_SATELLITES list contains dictionaries with various keys. However, there is no guarantee that all dictionaries will have the same keys, which may lead to a KeyError when accessing a specific key.

Suggested Fix:
Add error handling or ensure that all dictionaries in the GEO_RELAY_SATELLITES list have the same keys to prevent potential KeyErrors.

---

14. 🐛 Potential KeyError in ERROR_CODES dictionary

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/errors.py
Location	error_response function
Confidence	95%

Problem:
If the provided code is not found in the ERROR_CODES dictionary, a KeyError will be raised. This could lead to an unhandled exception and potentially reveal sensitive information.

Suggested Fix:
Add a try-except block to handle the KeyError and provide a default error response.

---

15. 🐛 In-flight request gauge not decremented on error paths

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/metrics.py
Location	init_metrics (Flask before/after request hooks)
Confidence	94%

Problem:
The before_request hook increments http_requests_in_flight, but the after_request hook (which decrements the gauge) is only executed for successful responses. If a request raises an exception and Flask returns an error response without invoking after_request, the gauge will remain elevated, leading to inaccurate in‑flight request counts.

Suggested Fix:
Add a teardown_request handler that always decrements http_requests_in_flight (checking for None) to ensure the gauge is balanced regardless of success or error.

---

16. 🐛 Unnecessary increment of TransmissionRequest._id_counter during snapshot restore

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/snapshot.py
Location	_rebuild_requests method (construction of TransmissionRequest)
Confidence	94%

Problem:
When rebuilding requests, the code creates a new TransmissionRequest instance for each dict. The constructor increments the class‑wide _id_counter, but the restore() method later overwrites the counter with the snapshot's saved value. This temporary increment can cause duplicate IDs if other parts of the system query the counter before it is reset, or if the constructor has side‑effects that depend on the counter value.

Suggested Fix:
Create a lightweight placeholder object without invoking the constructor that mutates the counter, or temporarily suppress the counter increment. One approach is to use TransmissionRequest.new(TransmissionRequest) to allocate the instance and then assign fields manually:

req = TransmissionRequest.__new__(TransmissionRequest)
# assign all attributes directly from the dict
req.id = d.get("id")
req.data_type = d.get("data_type", "DATA_SLICE")
... (set remaining fields) 

Alternatively, reset the counter before constructing any requests:

TransmissionRequest._id_counter = int(snapshot.get("id_counter", 0))
engine.transmission_requests = SnapshotManager._rebuild_requests(...)

Ensuring the counter is set first eliminates the unintended side‑effect.

---

17. 🐛 Potential exception in _snapshot_loop

Field	Details
Severity	🟡 Medium
Type	Bug
File	backend/stream.py
Location	_snapshot_loop
Confidence	92%

Problem:
The _snapshot_loop function catches all exceptions and ignores them. This can lead to unexpected behavior if an exception occurs. It would be better to log the exception or handle it in a more specific way.

Suggested Fix:
Consider logging the exception or handling it in a more specific way, such as restarting the loop or notifying the user.

---

About this report

This report was generated using Llama 3.3 70B.
Only findings with ≥80% confidence are included.
False positives are possible — use your own judgment.