Fix MQTT ticket initialization race condition in lane viewer#160
Open
tyronechrisharris wants to merge 13 commits into
Open
Fix MQTT ticket initialization race condition in lane viewer#160tyronechrisharris wants to merge 13 commits into
tyronechrisharris wants to merge 13 commits into
Conversation
Ensure that the MQTT ticket is always fetched and applied to the shared MQTT options before any realtime datasources are created or connected. This prevents a race condition where the first MQTT connection was being established with stale or empty credentials, leading to broken data streams in the lane view. - Updated Node.ts to always await ensureMqttTicket() in fetch methods. - Added prepareMqtt() to INode interface and Node class. - Updated DataSourceContext.tsx to await node.prepareMqtt() before initializing lane datasources. - Enhanced logging in Node.ts and OshAuthenticator for better observability of the ticket flow. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
Contributor
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
Ensures that the application fails closed if a valid MQTT ticket cannot be obtained. This prevents the creation of unauthenticated MQTT connections that would lead to broken live data streams and potential poisoning of the shared connector cache. - Updated Node.ts to make prepareMqtt() return a boolean success status and removed debug logging. - Modified DataSourceContext.tsx to skip datasource creation if prepareMqtt() fails. - Added security guards in LaneCollection.ts to refuse creating MQTT datasources without a valid __mqtt_ticket__ username and password. - Updated OshAuthenticator.java with safer password decoding logic and added logging for ticket auth success/failure. - Verified that all data fetching methods await the MQTT ticket to guarantee valid credentials. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit ensures that the live lane viewer correctly authenticated with the MQTT broker before creating datasources. It addresses a race condition where the first shared MQTT connector could be created without valid ticket credentials. - Node.ts: Added prepareMqtt() with strict validation of username, password, and endpoints. ensureMqttTicket() now always awaits valid credentials. - DataSourceContext.tsx: Fixed syntax (continue -> return) and ensured node.prepareMqtt() is awaited before creating lane datasources. - LaneCollection.ts: Added security guards to prevent datasource creation if MQTT ticket credentials are invalid or missing. - OshAuthenticator.java: Safely decode passwords using toString().toCharArray() and added logging for authentication results. - General: Removed debug logs and ensured "fail-closed" behavior for unauthorized MQTT access. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit addresses the missing live data in the lane viewer by ensuring proper sequencing and validation of MQTT ticket credentials before any realtime connections are established. - Node.ts: prepareMqtt() now strictly validates username, password, endpointUrl, and mqttPath before returning a success boolean. - DataSourceContext.tsx: Fixed syntax error (replaced continue with return) and ensured that datasource creation is skipped if MQTT preparation fails. - LaneCollection.ts: Implemented security guards in all datasource factory methods to refuse instantiation without valid __mqtt_ticket__ credentials. - OshAuthenticator.java: Updated password derivation to use a safer string-to- char-array conversion and added logging for ticket authentication results. - General: Removed debug logs and ensured the application fails closed for realtime data when authorization is missing or invalid. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This PR addresses the missing live data in the lane viewer by ensuring proper sequencing of MQTT ticket acquisition and fixing reverse proxy link generation. - RestApiServlet.java: Updated getApiRootURL() to honor X-Forwarded-Proto and X-Forwarded-Host headers. This prevents internal port leakage (e.g. :8282) in public links and MQTT ticket URLs. - Node.ts: Added prepareMqtt() with strict validation of ticket credentials before allowing connection. Improved proactive refresh with logging. - DataSourceContext.tsx: Guaranteed that node.prepareMqtt() is awaited before creating lane datasources. - LaneCollection.ts: Added fail-closed guards to all datasource factory methods to refuse instantiation without valid MQTT ticket credentials. - OshAuthenticator.java: Implemented safer password derivation and added authentication success/failure logging. These changes ensure live plot, alarm, and occupancy data arrive reliably and that navigation links work correctly behind reverse proxies. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This PR provides a multi-layered fix to restore live data in the lane viewer and adds comprehensive backend instrumentation to diagnose future pipeline failures. Frontend Changes: - Node.ts: Added a mandatory prepareMqtt() step that validates ticket credentials (username, password, endpoint) before allowing connection. - DataSourceContext.tsx: Guaranteed node.prepareMqtt() is awaited before creating lane datasources. - LaneCollection.ts: Added security guards to refuse creating MQTT datasources without valid __mqtt_ticket__ credentials. Backend Changes: - RestApiServlet.java: Updated getApiRootURL() to honor X-Forwarded-Proto and X-Forwarded-Host headers, fixing internal port leakage (:8282) in generated links. - OSCARServiceModule.java: Implemented a scheduled diagnostic table that logs status, UID, observation counts, and video clip counts for all lanes 30 seconds after startup. - Rs350OutputToOccupancy.java: Added logging for datastream checks, eventbus subscriptions, and report processing. - MessageHandler.java: Instrumented Rapiscan driver with raw line and occupancy trigger logging. - OccupancyWrapper.java: Instrumented state transitions and FFmpeg recording commands (CMD_OPEN_FILE/CMD_CLOSE_FILE). - OshAuthenticator.java: Safely decode passwords using a string-to-char- array conversion and added auth result logging. These changes ensure the live plots, alarms, and occupancy updates arrive reliably and provide the visibility needed to debug ingest issues in high-density deployments. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit addresses the loss of live data in the lane viewer by ensuring correct synchronization of MQTT credentials and adding the necessary backend visibility to diagnose pipeline failures. - RestApiServlet.java: Fixed reverse proxy port leakage (:8282) by honoring X-Forwarded-Proto and X-Forwarded-Host headers. - Node.ts: Mandatory prepareMqtt() ensures valid tickets are applied to shared options before connection. - Utilities.ts/LaneCollection.ts: Hardened datastream classification using robust array searches for observed property definitions. - OSCARServiceModule.java: Added a 5-minute repeating diagnostic table with recursive clip counting and missing-output warnings. - Ingest & Bridge: Added 60s rate-limited heartbeats to the Rapiscan, Rs350, and MQTT-to-REST bridge paths to track message flow. - OshAuthenticator.java: Safely decode passwords and log auth results. These changes resolve connection-layer issues and provide the observability required to pinpoint outages in the backend ingest pipeline. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit addresses the loss of live data in the lane viewer by ensuring correct synchronization of MQTT credentials and adding the necessary backend visibility to diagnose pipeline failures. - RestApiServlet.java: Fixed reverse proxy port leakage (:8282) by honoring X-Forwarded-Proto and X-Forwarded-Host headers. - Node.ts: Mandatory prepareMqtt() ensures valid tickets are applied to shared options before connection. - Utilities.ts/LaneCollection.ts: Hardened datastream classification using robust array searches for observed property definitions. - OSCARServiceModule.java: Added a 5-minute repeating diagnostic table with recursive clip counting (robust folder layout) and warnings. - Ingest & Bridge: Added 60s rate-limited heartbeats to the Rapiscan, Rs350, and MQTT-to-REST bridge paths to track message flow. - OshAuthenticator.java: Safely decode passwords and log auth results. - ObsHandler.java: Fixed compilation for observation delivery logging. These changes resolve connection-layer issues and provide the observability required to pinpoint outages in the backend ingest pipeline. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit fixes a regression where the live lane viewer stopped receiving plot, alarm, and occupancy data after the introduction of MQTT tickets.
Key changes:
- Frontend (web/oscar-viewer):
- Node.ts: Added `prepareMqtt()` to ensure tickets are fetched and applied to `mqttOpts` before any connection attempt.
- DataSourceContext.tsx: Explicitly await `node.prepareMqtt()` before creating datasources.
- LaneCollection.ts: Added 'fail-closed' guards to prevent MQTT connections without a valid `__mqtt_ticket__` user.
- Utilities.ts: Refactored datastream classification to use definition searches instead of brittle index-based checks.
- Backend (include/osh-core, include/osh-addons, include/osh-oakridge-modules):
- RestApiServlet.java: Added support for `X-Forwarded-Proto` and `X-Forwarded-Host` to avoid internal port leakage (e.g. :8282) in proxy environments.
- OSCARServiceModule.java: Implemented a periodic diagnostic table (every 5m) to track RPM UIDs, observation counts, and video clips.
- Ingest & Bridge Instrumentation: Added 60s rate-limited heartbeats to `Rs350OutputToOccupancy`, `MessageHandler`, `ConSysApiMqttConnector`, and `ObsHandler` to monitor data flow from sensor to broker.
- OshAuthenticator.java: Hardened ticket password decoding for improved reliability.
These changes ensure that the shared MQTT connector is never poisoned with stale credentials and that absolute API links remain reachable through the reverse proxy.
Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit fixes several issues blocking the live lane viewer data path: 1. Backend: Fixed malformed MQTT ticket URL generation in RestApiServlet.java. It now correctly extracts the path from the configured rootUrl when X-Forwarded headers are present, avoiding "null" strings and preserving the /sensorhub context. 2. Frontend: Added defensive fallback in Node.ts to strip "null" from ticket URLs and ensure the endpoint path defaults to oshPathRoot if the backend returns a bare root URL. 3. Frontend: Fixed MQTT timing by ensuring prepareMqtt() is awaited before creating realtime datasources, preventing the use of unauthenticated connectors. 4. Backend: Added extensive diagnostics and heartbeats across the ingest pipeline (Rapiscan driver, Rs350 occupancy, MQTT bridge) to improve visibility into data flow issues. 5. Frontend: Hardened datastream classification to use property-based searching instead of brittle index-based checks. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This commit fixes several critical issues blocking the live lane viewer: 1. Backend: Hardened RestApiServlet.getApiRootURL() to correctly generate public URLs when behind a reverse proxy. It now honors X-Forwarded-* headers (with fallbacks to Scheme/Host) and uses the configured path component to avoid context loss and "null" string leakage. 2. Frontend: Added defensive fallback in Node.ts to strip trailing "null" from malformed backend ticket URLs and ensures the endpoint path defaults to oshPathRoot. 3. Frontend: Fixed MQTT timing by ensuring prepareMqtt() is successfully awaited before any realtime datasources are created, preventing unauthenticated connector caching. 4. Backend: Added extensive diagnostics and heartbeats across the ingest pipeline (Rapiscan driver, Rs350 occupancy, MQTT bridge, and ObsHandler) to improve visibility into data production. 5. Documentation: Updated SYSTEM_ARCHITECTURE.md, SECURITY_ARCHITECTURE.md, and changelog.md to reflect the authenticated MQTT flow and network hardening. These changes ensure that the MQTT shared connector is correctly initialized with ticket credentials and that all telemetry traffic flows through the intended authenticated proxy paths. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
…ostics This commit fixes several critical issues blocking the live lane viewer: 1. Backend: Hardened RestApiServlet.getApiRootURL() to correctly generate public URLs when behind a reverse proxy. It now honors X-Forwarded-* headers (falling back to Scheme/Host) and strictly extracts the path component from the configured rootUrl to avoid context loss and "null" string leakage. 2. Frontend: Added defensive fallback in Node.ts to strip trailing "null" from malformed backend ticket URLs and ensures the WebSocket endpoint path defaults to oshPathRoot if the parsed path is empty. 3. Frontend: Fixed MQTT timing by ensuring prepareMqtt() is successfully awaited before any realtime datasources are created, preventing unauthenticated connector caching. Concurrent lane initialization now reuses a single ticket request per Node. 4. Backend: Added extensive diagnostics and heartbeats across the ingest pipeline (Rapiscan driver, Rs350 occupancy, MQTT bridge, and ObsHandler) to improve visibility into data production. 5. Documentation: Updated SYSTEM_ARCHITECTURE.md, SECURITY_ARCHITECTURE.md, and changelog.md to standardize on the /mqtt-ticket path and clarify the authenticated MQTT flow and performance optimizations. These changes ensure that the MQTT shared connector is correctly initialized with ticket credentials and that all telemetry traffic flows through the intended authenticated proxy paths. Co-authored-by: tyronechrisharris <142608718+tyronechrisharris@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses a critical regression where the live lane viewer would stop receiving plot data, alarms, and occupancy updates after the implementation of the MQTT ticket flow.
The root cause was identified as a race condition during application startup. The application was creating MQTT-backed datasources and establishing the first shared MQTT connection before the MQTT ticket fetch had completed. Because the MQTT connector is cached by endpoint URL, subsequent attempts to use the ticket credentials were being ignored as they reused the initial, unauthenticated connector.
Key changes:
prepareMqtt()step to theNodeinitialization process. This ensuresensureMqttTicket()is called and its result is applied to the sharedmqttOptsobject before any MQTT connections are attempted.fetch...methods inNode.tsto always await a valid ticket. Removed the conditional check that skipped ticket fetching if basic auth was present, ensuring that the preferred ticket mechanism is always used if available.DataSourceContext.tsxto explicitly awaitnode.prepareMqtt()before callingaddDefaultConSysApis()for discovered lanes.Fixes #159
PR created automatically by Jules for task 7802448099485375207 started by @tyronechrisharris