From b87ac28da6d22d998a5d2e4534bd24cd8e3970d9 Mon Sep 17 00:00:00 2001 From: Jonathan Walters Date: Mon, 3 Aug 2026 14:50:31 -0400 Subject: [PATCH 1/7] Add command metadata propagation --- docs.openc3.com/docs/guides/scripting-api.md | 3 +++ openc3/lib/openc3/api/cmd_api.rb | 7 +++++- .../microservices/interface_microservice.rb | 3 +++ .../microservices/queue_microservice.rb | 4 +-- openc3/lib/openc3/models/queue_model.rb | 17 +++++++------ openc3/lib/openc3/script/commands.rb | 6 ++--- openc3/lib/openc3/topics/command_topic.rb | 4 +++ openc3/python/openc3/api/cmd_api.py | 7 +++++- .../microservices/interface_microservice.py | 2 ++ openc3/python/openc3/models/queue_model.py | 11 +++++++- openc3/python/openc3/script/commands.py | 3 +++ openc3/python/openc3/topics/command_topic.py | 7 ++++++ openc3/python/test/api/test_cmd_api.py | 19 ++++++++++++++ .../test_interface_microservice.py | 2 ++ openc3/python/test/models/test_queue_model.py | 20 +++++++++++++++ openc3/python/test/script/test_commands.py | 6 +++++ openc3/spec/api/cmd_api_spec.rb | 12 +++++++++ .../interface_microservice_spec.rb | 5 +++- .../microservices/queue_microservice_spec.rb | 25 +++++++++++-------- openc3/spec/models/queue_model_spec.rb | 6 +++-- openc3/spec/script/commands_spec.rb | 10 ++++++++ 21 files changed, 150 insertions(+), 29 deletions(-) diff --git a/docs.openc3.com/docs/guides/scripting-api.md b/docs.openc3.com/docs/guides/scripting-api.md index b1fc52f5fc..d1874bb32f 100644 --- a/docs.openc3.com/docs/guides/scripting-api.md +++ b/docs.openc3.com/docs/guides/scripting-api.md @@ -840,6 +840,7 @@ cmd("", "", "Param #1 Name" => , "Par | timeout | Optional named parameter to change the default timeout value of 5 seconds | | log_message | Optional named parameter to prevent logging of the command | | validate | Optional named parameter to enable/disable validation (default is True) | +| extra | Optional metadata Hash/dict carried with the command packet to the interface | @@ -848,6 +849,7 @@ cmd("", "", "Param #1 Name" => , "Par cmd("INST COLLECT with DURATION 10, TYPE NORMAL") cmd("INST", "COLLECT", { "DURATION": 10, "TYPE": "NORMAL" }) cmd("INST ABORT", timeout=10, log_message=False, validate=False) +cmd("INST ABORT", extra={ "request_id": "1234-5678" }) ``` @@ -860,6 +862,7 @@ cmd("INST COLLECT with DURATION 10, TYPE NORMAL") cmd("INST", "COLLECT", "DURATION" => 10, "TYPE" => "NORMAL") cmd("INST", "COLLECT", { "DURATION" => 10, "TYPE" => "NORMAL" }) cmd("INST ABORT", timeout: 10, log_message: false, validate: false) +cmd("INST ABORT", extra: { "request_id" => "1234-5678" }) ``` diff --git a/openc3/lib/openc3/api/cmd_api.rb b/openc3/lib/openc3/api/cmd_api.rb index 6a159c3794..1f82043c73 100644 --- a/openc3/lib/openc3/api/cmd_api.rb +++ b/openc3/lib/openc3/api/cmd_api.rb @@ -450,7 +450,7 @@ def _extract_target_command_parameter_names(method_name, *args) # NOTE: When adding new keywords to this method, make sure to update script/commands.rb def _cmd_implementation(method_name, *args, range_check:, hazardous_check:, raw:, timeout: nil, log_message: nil, manual: false, validate: true, queue: nil, - queue_username: nil, scope: $openc3_scope, token: $openc3_token, **kwargs) + queue_username: nil, extra: nil, scope: $openc3_scope, token: $openc3_token, **kwargs) extract_string_kwargs_to_args(args, kwargs) unless [nil, true, false].include?(log_message) raise "Invalid log_message parameter: #{log_message}. Must be true or false." @@ -462,6 +462,9 @@ def _cmd_implementation(method_name, *args, range_check:, hazardous_check:, raw: raise "Invalid timeout parameter: #{timeout}. Must be numeric." end end + unless extra.nil? || extra.is_a?(Hash) + raise "Invalid extra parameter: #{extra}. Must be a Hash." + end case args.length when 1 @@ -535,6 +538,7 @@ def _cmd_implementation(method_name, *args, range_check:, hazardous_check:, raw: 'log_message' => log_message.to_s, 'obfuscated_items' => packet['obfuscated_items'].to_s } + command['extra'] = extra unless extra.nil? # Record the original queuing user (author) separately from 'username' (the # user or process that actually executed the command). Command History shows # 'username' as "Executed By" and queue_username as "Queued By". @@ -550,6 +554,7 @@ def _cmd_implementation(method_name, *args, range_check:, hazardous_check:, raw: target_name: target_name, cmd_name: cmd_name, cmd_params: cmd_params, + extra: extra, validate: validate, timeout: timeout, username: username, diff --git a/openc3/lib/openc3/microservices/interface_microservice.rb b/openc3/lib/openc3/microservices/interface_microservice.rb index 86dfdea240..148b39b8d0 100644 --- a/openc3/lib/openc3/microservices/interface_microservice.rb +++ b/openc3/lib/openc3/microservices/interface_microservice.rb @@ -284,6 +284,9 @@ def run end command.extra ||= {} + if msg_hash['extra'] + command.extra.merge!(JSON.parse(msg_hash['extra'], allow_nan: true, create_additions: true)) + end command.extra['cmd_string'] = msg_hash['cmd_string'] command.extra['username'] = msg_hash['username'] command.extra['interface_name'] = @interface.name diff --git a/openc3/lib/openc3/microservices/queue_microservice.rb b/openc3/lib/openc3/microservices/queue_microservice.rb index a20066ec1d..e39c04bccd 100644 --- a/openc3/lib/openc3/microservices/queue_microservice.rb +++ b/openc3/lib/openc3/microservices/queue_microservice.rb @@ -78,12 +78,12 @@ def process_queued_commands timeout = command['timeout'] # Pass queue_username so Command History is attributed to the original # author rather than the queue microservice name - cmd(command['target_name'], command['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command['username'], scope: @scope) + cmd(command['target_name'], command['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: command['extra'], scope: @scope) elsif command['value'] # Legacy format: use single string parameter for backwards compatibility validate = command.key?('validate') ? command['validate'] : true timeout = command['timeout'] - cmd(command['value'], queue: false, validate: validate, timeout: timeout, queue_username: command['username'], scope: @scope) + cmd(command['value'], queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: command['extra'], scope: @scope) else @logger.error "QueueProcessor: Invalid command format, missing required fields" end diff --git a/openc3/lib/openc3/models/queue_model.rb b/openc3/lib/openc3/models/queue_model.rb index aa71747ebf..1b01a92573 100644 --- a/openc3/lib/openc3/models/queue_model.rb +++ b/openc3/lib/openc3/models/queue_model.rb @@ -40,7 +40,7 @@ def self.all(scope:) end # END NOTE - def self.queue_command(name, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, validate: true, timeout: nil, username:, scope:) + def self.queue_command(name, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, extra: nil, validate: true, timeout: nil, username:, scope:) model = get_model(name: name, scope: scope) raise QueueError, "Queue '#{name}' not found in scope '#{scope}'" unless model @@ -53,7 +53,7 @@ def self.queue_command(name, command: nil, target_name: nil, cmd_name: nil, cmd_ id = result.empty? ? 1.0 : result[0][1].to_f + 1 command_data = build_command_data(username: username, command: command, target_name: target_name, - cmd_name: cmd_name, cmd_params: cmd_params, validate: validate, timeout: timeout) + cmd_name: cmd_name, cmd_params: cmd_params, extra: extra, validate: validate, timeout: timeout) Store.zadd("#{scope}:#{name}", id, command_data.to_json) model.notify(kind: 'command') end @@ -61,7 +61,7 @@ def self.queue_command(name, command: nil, target_name: nil, cmd_name: nil, cmd_ # Build the hash that gets serialized into Redis. Always uses symbol keys so # downstream code in this class can access values consistently. cmd_params is # JSON-encoded as a string so binary data survives the round-trip via as_json. - def self.build_command_data(username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, validate: nil, timeout: nil) + def self.build_command_data(username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, extra: nil, validate: nil, timeout: nil) command_data = { username: username, timestamp: Time.now.to_nsec_from_epoch } if target_name && cmd_name command_data[:target_name] = target_name @@ -74,6 +74,7 @@ def self.build_command_data(username:, command: nil, target_name: nil, cmd_name: end command_data[:validate] = validate unless validate.nil? command_data[:timeout] = timeout unless timeout.nil? + command_data[:extra] = extra unless extra.nil? command_data end @@ -118,7 +119,7 @@ def notify(kind:) QueueTopic.write_notification(notification, scope: @scope) end - def insert_command(id: nil, username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, validate: nil, timeout: nil) + def insert_command(id: nil, username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, extra: nil, validate: nil, timeout: nil) if @state == 'DISABLE' command_name = command || "#{target_name} #{cmd_name}" raise QueueError, "Queue '#{@name}' is disabled. Command '#{command_name}' not queued." @@ -130,12 +131,12 @@ def insert_command(id: nil, username:, command: nil, target_name: nil, cmd_name: end command_data = self.class.build_command_data(username: username, command: command, target_name: target_name, - cmd_name: cmd_name, cmd_params: cmd_params, validate: validate, timeout: timeout) + cmd_name: cmd_name, cmd_params: cmd_params, extra: extra, validate: validate, timeout: timeout) Store.zadd("#{@scope}:#{@name}", id, command_data.to_json) notify(kind: 'command') end - def update_command(id:, username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, validate: nil, timeout: nil) + def update_command(id:, username:, command: nil, target_name: nil, cmd_name: nil, cmd_params: nil, extra: nil, validate: nil, timeout: nil) if @state == 'DISABLE' raise QueueError, "Queue '#{@name}' is disabled. Command at id #{id} not updated." end @@ -147,7 +148,7 @@ def update_command(id:, username:, command: nil, target_name: nil, cmd_name: nil Store.zremrangebyscore("#{@scope}:#{@name}", id, id) command_data = self.class.build_command_data(username: username, command: command, target_name: target_name, - cmd_name: cmd_name, cmd_params: cmd_params, validate: validate, timeout: timeout) + cmd_name: cmd_name, cmd_params: cmd_params, extra: extra, validate: validate, timeout: timeout) Store.zadd("#{@scope}:#{@name}", id, command_data.to_json) notify(kind: 'command') end @@ -243,4 +244,4 @@ def destroy super() end end -end \ No newline at end of file +end diff --git a/openc3/lib/openc3/script/commands.rb b/openc3/lib/openc3/script/commands.rb index 258a2c3ee1..f8e440c394 100644 --- a/openc3/lib/openc3/script/commands.rb +++ b/openc3/lib/openc3/script/commands.rb @@ -117,7 +117,7 @@ def _cmd_disconnect(cmd, raw, no_range, no_hazardous, *args, scope: $openc3_scop # except for range_check, hazardous_check, and raw as they are part of the cmd name # manual is always false since this is called from script and that is the default # NOTE: This is a helper method and should not be called directly - def _cmd(cmd, cmd_no_hazardous, *args, timeout: nil, log_message: nil, validate: true, queue: nil, scope: $openc3_scope, token: $openc3_token, **kwargs) + def _cmd(cmd, cmd_no_hazardous, *args, timeout: nil, log_message: nil, validate: true, queue: nil, extra: nil, scope: $openc3_scope, token: $openc3_token, **kwargs) extract_string_kwargs_to_args(args, kwargs) raw = cmd.include?('raw') no_range = cmd.include?('no_range') || cmd.include?('no_checks') @@ -127,7 +127,7 @@ def _cmd(cmd, cmd_no_hazardous, *args, timeout: nil, log_message: nil, validate: else begin begin - command = $api_server.method_missing(cmd, *args, timeout: timeout, log_message: log_message, validate: validate, queue: queue, scope: scope, token: token) + command = $api_server.method_missing(cmd, *args, timeout: timeout, log_message: log_message, validate: validate, queue: queue, extra: extra, scope: scope, token: token) if log_message.nil? or log_message _log_cmd(command, raw, no_range, no_hazardous) end @@ -135,7 +135,7 @@ def _cmd(cmd, cmd_no_hazardous, *args, timeout: nil, log_message: nil, validate: # This opens a prompt at which point they can cancel and stop the script # or say Yes and send the command. Thus we don't care about the return value. prompt_for_hazardous(e.target_name, e.cmd_name, e.hazardous_description) - command = $api_server.method_missing(cmd_no_hazardous, *args, timeout: timeout, log_message: log_message, validate: validate, queue: queue, scope: scope, token: token) + command = $api_server.method_missing(cmd_no_hazardous, *args, timeout: timeout, log_message: log_message, validate: validate, queue: queue, extra: extra, scope: scope, token: token) if log_message.nil? or log_message _log_cmd(command, raw, no_range, no_hazardous) end diff --git a/openc3/lib/openc3/topics/command_topic.rb b/openc3/lib/openc3/topics/command_topic.rb index 08eb57f11b..c63dcf513f 100644 --- a/openc3/lib/openc3/topics/command_topic.rb +++ b/openc3/lib/openc3/topics/command_topic.rb @@ -45,6 +45,8 @@ def self.send_command(command, timeout: COMMAND_ACK_TIMEOUT_S, scope:, obfuscate # Save the existing cmd_params Hash and JSON generate before writing to the topic cmd_params = command['cmd_params'] command['cmd_params'] = JSON.generate(command['cmd_params'].as_json, allow_nan: true) + extra = command['extra'] + command['extra'] = JSON.generate(extra.as_json, allow_nan: true) if extra OpenC3.inject_context(command) db_shard = Store.db_shard_for_target(command['target_name'], scope: scope) @@ -53,6 +55,7 @@ def self.send_command(command, timeout: COMMAND_ACK_TIMEOUT_S, scope:, obfuscate if timeout <= 0 Topic.write_topic("{#{scope}__CMD}TARGET__#{command['target_name']}", command, '*', 100, db_shard: db_shard) command["cmd_params"] = cmd_params # Restore the original cmd_params Hash + command['extra'] = extra if extra return command end @@ -60,6 +63,7 @@ def self.send_command(command, timeout: COMMAND_ACK_TIMEOUT_S, scope:, obfuscate Topic.update_topic_offsets([ack_topic], db_shard: db_shard) cmd_id = Topic.write_topic("{#{scope}__CMD}TARGET__#{command['target_name']}", command, '*', 100, db_shard: db_shard) command["cmd_params"] = cmd_params # Restore the original cmd_params Hash + command['extra'] = extra if extra time = Time.now while (Time.now - time) < timeout Topic.read_topics([ack_topic], db_shard: db_shard) do |_topic, _msg_id, msg_hash, _redis| diff --git a/openc3/python/openc3/api/cmd_api.py b/openc3/python/openc3/api/cmd_api.py index 40c097d8a4..8ed6551ce0 100644 --- a/openc3/python/openc3/api/cmd_api.py +++ b/openc3/python/openc3/api/cmd_api.py @@ -610,6 +610,9 @@ def _cmd_implementation( manual=manual, ) queue_username = kwargs.get("queue_username") + extra = kwargs.get("extra") + if extra is not None and not isinstance(extra, dict): + raise RuntimeError(f"Invalid extra parameter: {extra}. Must be a dict.") if not user: user = {} user["username"] = os.environ.get("OPENC3_MICROSERVICE_NAME") @@ -689,6 +692,8 @@ def _cmd_implementation( "log_message": str(log_message), "obfuscated_items": json.dumps(packet.get("obfuscated_items", [])), } + if extra is not None: + command["extra"] = extra # Record the original queuing user (author) separately from 'username' (the # user or process that actually executed the command). Command History shows # 'username' as "Executed By" and queue_username as "Queued By". @@ -704,7 +709,7 @@ def _cmd_implementation( # Pull the command out of the script string, e.g. cmd("INST ABORT") queued = cmd_string.split('("')[1].split('")')[0] QueueModel.queue_command( - queue, command=queued, username=username, scope=scope, validate=validate, timeout=timeout + queue, command=queued, username=username, scope=scope, validate=validate, timeout=timeout, extra=extra ) else: CommandTopic.send_command(command, timeout=timeout, scope=scope) diff --git a/openc3/python/openc3/microservices/interface_microservice.py b/openc3/python/openc3/microservices/interface_microservice.py index ebdf5a9b9f..c0fe5a7422 100644 --- a/openc3/python/openc3/microservices/interface_microservice.py +++ b/openc3/python/openc3/microservices/interface_microservice.py @@ -321,6 +321,8 @@ def process_cmd(self, topic, msg_id, msg_hash, _redis): return str(e) command.extra = command.extra or {} + if msg_hash.get(b"extra"): + command.extra.update(json.loads(msg_hash[b"extra"], cls=JsonDecoder)) command.extra["cmd_string"] = msg_hash.get(b"cmd_string", b"").decode() command.extra["username"] = msg_hash.get(b"username", b"").decode() command.extra["interface_name"] = self.interface.name diff --git a/openc3/python/openc3/models/queue_model.py b/openc3/python/openc3/models/queue_model.py index 9d75134eca..4e1f78c2a9 100644 --- a/openc3/python/openc3/models/queue_model.py +++ b/openc3/python/openc3/models/queue_model.py @@ -48,7 +48,14 @@ def all(cls, scope: str): # However we need a lot of methods to enable cls.get_model and model.notify @classmethod def queue_command( - cls, name: str, command: str, username: str, scope: str, validate: bool = True, timeout: float = None + cls, + name: str, + command: str, + username: str, + scope: str, + validate: bool = True, + timeout: float = None, + extra: dict | None = None, ): model = cls.get_model(name=name, scope=scope) if not model: @@ -68,6 +75,8 @@ def queue_command( "timeout": timeout, "timestamp": time.time_ns(), } + if extra is not None: + command_data["extra"] = extra Store.zadd(f"{scope}:{name}", {json.dumps(command_data): index}) model.notify(kind="command") else: diff --git a/openc3/python/openc3/script/commands.py b/openc3/python/openc3/script/commands.py index 59579688c9..97960c928e 100644 --- a/openc3/python/openc3/script/commands.py +++ b/openc3/python/openc3/script/commands.py @@ -228,6 +228,7 @@ def _cmd( log_message=None, validate=True, queue=None, + extra=None, scope=OPENC3_SCOPE, ): """Send the command and log the results @@ -249,6 +250,7 @@ def _cmd( log_message=log_message, validate=validate, queue=queue, + extra=extra, scope=scope, ) if log_message is None or log_message: @@ -270,6 +272,7 @@ def _cmd( log_message=log_message, validate=validate, queue=queue, + extra=extra, scope=scope, ) if log_message is None or log_message: diff --git a/openc3/python/openc3/topics/command_topic.py b/openc3/python/openc3/topics/command_topic.py index a76c07fa2e..8fa32a403c 100644 --- a/openc3/python/openc3/topics/command_topic.py +++ b/openc3/python/openc3/topics/command_topic.py @@ -57,6 +57,9 @@ def send_command(cls, command, timeout, scope, obfuscated_items=None): # Save the existing cmd_params Hash and JSON generate before writing to the topic cmd_params = command["cmd_params"] command["cmd_params"] = json.dumps(command["cmd_params"], cls=JsonEncoder) + extra = command.get("extra") + if extra is not None: + command["extra"] = json.dumps(extra, cls=JsonEncoder) db_shard = Store.db_shard_for_target(command["target_name"], scope=scope) @@ -70,6 +73,8 @@ def send_command(cls, command, timeout, scope, obfuscated_items=None): db_shard=db_shard, ) command["cmd_params"] = cmd_params # Restore the original cmd_params dict + if extra is not None: + command["extra"] = extra return command ack_topic = f"{{{scope}__ACKCMD}}TARGET__{command['target_name']}" @@ -82,6 +87,8 @@ def send_command(cls, command, timeout, scope, obfuscated_items=None): db_shard=db_shard, ) command["cmd_params"] = cmd_params # Restore the original cmd_params dict + if extra is not None: + command["extra"] = extra start_time = time.time() while (time.time() - start_time) < timeout: for _, _, msg_hash, _ in Topic.read_topics([ack_topic], db_shard=db_shard): diff --git a/openc3/python/test/api/test_cmd_api.py b/openc3/python/test/api/test_cmd_api.py index d4edaa22e9..9b79224ff8 100644 --- a/openc3/python/test/api/test_cmd_api.py +++ b/openc3/python/test/api/test_cmd_api.py @@ -247,6 +247,25 @@ def test_cmd_records_the_original_queuing_user(self): # username is the executing user/process, not the author self.assertNotEqual(command["username"], "original_author") + def test_cmd_carries_extra_metadata(self): + extra = {"flow_uuid": "1234-5678", "hv_id": 42} + for name in [ + "cmd", + "cmd_no_range_check", + "cmd_no_hazardous_check", + "cmd_no_checks", + "cmd_raw", + "cmd_raw_no_range_check", + "cmd_raw_no_hazardous_check", + "cmd_raw_no_checks", + ]: + command = globals()[name]("INST", "ABORT", extra=extra) + self.assertEqual(command["extra"], extra) + + def test_cmd_rejects_invalid_extra_metadata(self): + with self.assertRaisesRegex(RuntimeError, "Invalid extra parameter: invalid. Must be a dict"): + cmd("INST", "ABORT", extra="invalid") + def test_cmd_warns_about_required_parameters(self): for name in [ "cmd", diff --git a/openc3/python/test/microservices/test_interface_microservice.py b/openc3/python/test/microservices/test_interface_microservice.py index a5e640f814..386a3ef532 100644 --- a/openc3/python/test/microservices/test_interface_microservice.py +++ b/openc3/python/test/microservices/test_interface_microservice.py @@ -526,6 +526,7 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): b"hazardous_check": b"TRUE", b"cmd_string": b"cmd('INST ABORT')", b"username": b"test_user", + b"extra": json.dumps({"flow_uuid": "1234-5678", "username": "untrusted"}).encode(), b"queue_username": b"DEFAULT__MULTI__INST", b"validate": b"TRUE", b"manual": b"FALSE", @@ -537,6 +538,7 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): # queue_username must be copied into the command extra so Command History # can show "Queued By" for queued commands command = mock_write.call_args[0][0] + self.assertEqual(command.extra["flow_uuid"], "1234-5678") self.assertEqual(command.extra["username"], "test_user") self.assertEqual(command.extra.get("queue_username"), "DEFAULT__MULTI__INST") diff --git a/openc3/python/test/models/test_queue_model.py b/openc3/python/test/models/test_queue_model.py index c5cda6125b..ccc11353c7 100644 --- a/openc3/python/test/models/test_queue_model.py +++ b/openc3/python/test/models/test_queue_model.py @@ -95,6 +95,26 @@ def test_queue_command_validate_false_timeout_zero(self, mock_get_model, mock_st ) mock_store.zadd.assert_called_once_with("DEFAULT:TEST", {expected_data: 1.0}) + @patch("openc3.models.queue_model.Store") + @patch("openc3.models.queue_model.QueueModel.get_model") + def test_queue_command_with_extra_metadata(self, mock_get_model, mock_store): + mock_model = Mock() + mock_model.state = "RUNNING" + mock_get_model.return_value = mock_model + mock_store.zrevrange.return_value = [] + + with patch("time.time_ns", return_value=1234567890): + QueueModel.queue_command( + "TEST", + command="CMD", + username="user", + scope="DEFAULT", + extra={"flow_uuid": "1234-5678"}, + ) + + queued = json.loads(next(iter(mock_store.zadd.call_args.args[1]))) + self.assertEqual(queued["extra"], {"flow_uuid": "1234-5678"}) + @patch("openc3.models.queue_model.Store") @patch("openc3.models.queue_model.QueueModel.get_model") def test_queue_command_with_existing_items(self, mock_get_model, mock_store): diff --git a/openc3/python/test/script/test_commands.py b/openc3/python/test/script/test_commands.py index 13fa6b45eb..cc3207e5f9 100644 --- a/openc3/python/test/script/test_commands.py +++ b/openc3/python/test/script/test_commands.py @@ -149,6 +149,12 @@ def test_sends_a_cmd(self): ) self.assertEqual(gArgs, ("INST ABORT",)) + def test_sends_extra_metadata(self): + extra = {"flow_uuid": "1234-5678", "hv_id": 42} + for _stdout in capture_io(): + cmd("INST ABORT", extra=extra) + self.assertEqual(gKwargs["extra"], extra) + def test_sends_a_cmd_raw(self): global gArgs global gKwargs diff --git a/openc3/spec/api/cmd_api_spec.rb b/openc3/spec/api/cmd_api_spec.rb index caac8a7939..ad0ec06248 100644 --- a/openc3/spec/api/cmd_api_spec.rb +++ b/openc3/spec/api/cmd_api_spec.rb @@ -147,6 +147,18 @@ def test_cmd_unknown(method) expect(command['username']).to_not eql 'original_author' end + it "carries extra metadata" do + extra = { 'flow_uuid' => '1234-5678', 'hv_id' => 42 } + command = @api.send(method, "INST", "ABORT", extra: extra) + expect(command['extra']).to eql extra + end + + it "rejects invalid extra metadata" do + expect { @api.send(method, "INST", "ABORT", extra: 'invalid') }.to raise_error( + "Invalid extra parameter: invalid. Must be a Hash." + ) + end + it "warns about required parameters" do expect { @api.send(method, "INST COLLECT with DURATION 5") }.to raise_error(/Required/) end diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index 8715cd82be..816c03dec0 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -281,13 +281,16 @@ class ApiTest # queue_username is the original author (shown as "Queued By"), passed # by the queue microservice when a command is released from a queue - @api.cmd("INST", "ABORT", queue_username: "DEFAULT__MULTI__INST") + @api.cmd("INST", "ABORT", queue_username: "DEFAULT__MULTI__INST", + extra: { 'flow_uuid' => '1234-5678', 'username' => 'untrusted' }) sleep 0.01 im.shutdown expect(captured).to_not be_nil expect(captured.target_name).to eql("INST") expect(captured.packet_name).to eql("ABORT") + expect(captured.extra['flow_uuid']).to eql('1234-5678') + expect(captured.extra['username']).to_not eql('untrusted') expect(captured.extra['queue_username']).to eql("DEFAULT__MULTI__INST") end diff --git a/openc3/spec/microservices/queue_microservice_spec.rb b/openc3/spec/microservices/queue_microservice_spec.rb index 75fb777558..917d866710 100644 --- a/openc3/spec/microservices/queue_microservice_spec.rb +++ b/openc3/spec/microservices/queue_microservice_spec.rb @@ -86,7 +86,10 @@ module OpenC3 describe '#process_queued_commands' do let(:command1) { { 'username' => 'test_user', 'value' => 'cmd("TARGET", "COMMAND", {"PARAM": 1})' } } let(:command2) { { 'username' => 'test_user', 'value' => 'cmd("TARGET", "COMMAND2", {"PARAM": 2})' } } - let(:command3_new_format) { { 'username' => 'test_user', 'target_name' => 'TARGET', 'cmd_name' => 'COMMAND3', 'cmd_params' => JSON.generate({ 'PARAM' => 3 }) } } + let(:command3_new_format) do + { 'username' => 'test_user', 'target_name' => 'TARGET', 'cmd_name' => 'COMMAND3', + 'cmd_params' => JSON.generate({ 'PARAM' => 3 }), 'extra' => { 'flow_uuid' => '1234-5678' } } + end let(:command4_new_format) { { 'username' => 'test_user', 'target_name' => 'TARGET', 'cmd_name' => 'COMMAND4' } } before do @@ -114,9 +117,9 @@ module OpenC3 expect(Store).to have_received(:bzpopmin).exactly(3).times expect(processor).to have_received(:cmd) - .with(command1['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with(command1['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user', extra: nil) expect(processor).to have_received(:cmd) - .with(command2['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with(command2['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user', extra: nil) end it 'processes commands with new format (target_name, cmd_name, cmd_params)' do @@ -136,7 +139,8 @@ module OpenC3 expect(Store).to have_received(:bzpopmin).exactly(2).times expect(processor).to have_received(:cmd) - .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: nil, validate: true, + queue_username: 'test_user', extra: { 'flow_uuid' => '1234-5678' }) end it 'processes commands with new format without cmd_params' do @@ -156,7 +160,7 @@ module OpenC3 expect(Store).to have_received(:bzpopmin).exactly(2).times expect(processor).to have_received(:cmd) - .with('TARGET', 'COMMAND4', {}, queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with('TARGET', 'COMMAND4', {}, queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user', extra: nil) end it 'processes mixed legacy and new format commands' do @@ -178,9 +182,10 @@ module OpenC3 expect(Store).to have_received(:bzpopmin).exactly(3).times expect(processor).to have_received(:cmd) - .with(command1['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with(command1['value'], queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user', extra: nil) expect(processor).to have_received(:cmd) - .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: nil, validate: true, queue_username: 'test_user') + .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: nil, validate: true, + queue_username: 'test_user', extra: { 'flow_uuid' => '1234-5678' }) end it 'processes legacy command with validate false and timeout 0' do @@ -200,7 +205,7 @@ module OpenC3 processor.process_queued_commands expect(processor).to have_received(:cmd) - .with(command_no_validate['value'], queue: false, scope: scope, timeout: 0, validate: false, queue_username: 'test_user') + .with(command_no_validate['value'], queue: false, scope: scope, timeout: 0, validate: false, queue_username: 'test_user', extra: nil) end it 'processes new format command with validate false and timeout 0' do @@ -220,7 +225,7 @@ module OpenC3 processor.process_queued_commands expect(processor).to have_received(:cmd) - .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: 0, validate: false, queue_username: 'test_user') + .with('TARGET', 'COMMAND3', { 'PARAM' => 3 }, queue: false, scope: scope, timeout: 0, validate: false, queue_username: 'test_user', extra: nil) end it 'logs error for invalid command format (missing required fields)' do @@ -441,4 +446,4 @@ module OpenC3 end end end -end \ No newline at end of file +end diff --git a/openc3/spec/models/queue_model_spec.rb b/openc3/spec/models/queue_model_spec.rb index bfc1058ada..7b64cf61bc 100644 --- a/openc3/spec/models/queue_model_spec.rb +++ b/openc3/spec/models/queue_model_spec.rb @@ -152,7 +152,8 @@ module OpenC3 model.create allow(QueueTopic).to receive(:write_notification) - QueueModel.queue_command("TEST", target_name: "INST", cmd_name: "COLLECT", cmd_params: { "TYPE" => "NORMAL" }, username: 'test_user', scope: "DEFAULT") + QueueModel.queue_command("TEST", target_name: "INST", cmd_name: "COLLECT", cmd_params: { "TYPE" => "NORMAL" }, + extra: { "flow_uuid" => "1234-5678" }, username: 'test_user', scope: "DEFAULT") commands = Store.zrange("DEFAULT:TEST", 0, -1).map { |cmd| JSON.parse(cmd) } expect(commands).to contain_exactly({ @@ -160,6 +161,7 @@ module OpenC3 "target_name" => "INST", "cmd_name" => "COLLECT", "cmd_params" => "{\"TYPE\":\"NORMAL\"}", + "extra" => { "flow_uuid" => "1234-5678" }, "validate" => true, "timestamp" => anything }) @@ -888,4 +890,4 @@ module OpenC3 end end end -end \ No newline at end of file +end diff --git a/openc3/spec/script/commands_spec.rb b/openc3/spec/script/commands_spec.rb index ec64c46bf6..f7b590a063 100644 --- a/openc3/spec/script/commands_spec.rb +++ b/openc3/spec/script/commands_spec.rb @@ -32,6 +32,7 @@ class CommandsSpecApi include Extract include Api include Authorization + attr_reader :last_kw_params def shutdown end @@ -43,6 +44,7 @@ def generate_url end def method_missing(name, *params, **kw_params) + @last_kw_params = kw_params self.send(name, *params, **kw_params) end end @@ -139,6 +141,14 @@ def setup_cmd_handler_thread end end + if connect == 'connected' + it "sends extra metadata" do + extra = { 'flow_uuid' => '1234-5678', 'hv_id' => 42 } + capture_io { cmd("INST ABORT", extra: extra) } + expect(@api.last_kw_params[:extra]).to eql(extra) + end + end + it "raises without any parameters" do expect { cmd() }.to raise_error(/Invalid number of arguments/) end From b33b8271aac1a1c28675c1686b14478f9ad21684 Mon Sep 17 00:00:00 2001 From: Jonathan Walters Date: Thu, 27 Aug 2026 01:09:32 -0400 Subject: [PATCH 2/7] Fix command metadata audit and queue encoding --- .../lib/openc3/microservices/interface_microservice.rb | 3 +++ openc3/lib/openc3/microservices/queue_microservice.rb | 8 ++++++-- openc3/lib/openc3/models/queue_model.rb | 2 +- .../openc3/microservices/interface_microservice.py | 3 +++ openc3/python/openc3/models/queue_model.py | 3 ++- .../test/microservices/test_interface_microservice.py | 10 +++++++++- openc3/python/test/models/test_queue_model.py | 8 ++++++-- .../spec/microservices/interface_microservice_spec.rb | 6 +++++- openc3/spec/microservices/queue_microservice_spec.rb | 2 +- openc3/spec/models/queue_model_spec.rb | 5 +++-- 10 files changed, 39 insertions(+), 11 deletions(-) diff --git a/openc3/lib/openc3/microservices/interface_microservice.rb b/openc3/lib/openc3/microservices/interface_microservice.rb index 148b39b8d0..e28e1bfb7f 100644 --- a/openc3/lib/openc3/microservices/interface_microservice.rb +++ b/openc3/lib/openc3/microservices/interface_microservice.rb @@ -287,6 +287,9 @@ def run if msg_hash['extra'] command.extra.merge!(JSON.parse(msg_hash['extra'], allow_nan: true, create_additions: true)) end + # These fields are populated from COSMOS workflow state below. + command.extra.delete('queue_username') + command.extra.delete('approver') command.extra['cmd_string'] = msg_hash['cmd_string'] command.extra['username'] = msg_hash['username'] command.extra['interface_name'] = @interface.name diff --git a/openc3/lib/openc3/microservices/queue_microservice.rb b/openc3/lib/openc3/microservices/queue_microservice.rb index e39c04bccd..c4b19594a7 100644 --- a/openc3/lib/openc3/microservices/queue_microservice.rb +++ b/openc3/lib/openc3/microservices/queue_microservice.rb @@ -61,6 +61,10 @@ def process_queued_commands _queue_name, command_data, _timestamp = Store.bzpopmin("#{@scope}:#{@name}", timeout: 0.2) if command_data command = JSON.parse(command_data) + extra = command['extra'] + if extra.is_a?(String) + extra = JSON.parse(extra, allow_nan: true, create_additions: true) + end # It's important to set queue: false here to avoid infinite recursion when # OPENC3_DEFAULT_QUEUE is set because commands would be re-queued to the default queue # NOTE: cmd() via script rescues hazardous errors and calls prompt_for_hazardous() @@ -78,12 +82,12 @@ def process_queued_commands timeout = command['timeout'] # Pass queue_username so Command History is attributed to the original # author rather than the queue microservice name - cmd(command['target_name'], command['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: command['extra'], scope: @scope) + cmd(command['target_name'], command['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: extra, scope: @scope) elsif command['value'] # Legacy format: use single string parameter for backwards compatibility validate = command.key?('validate') ? command['validate'] : true timeout = command['timeout'] - cmd(command['value'], queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: command['extra'], scope: @scope) + cmd(command['value'], queue: false, validate: validate, timeout: timeout, queue_username: command['username'], extra: extra, scope: @scope) else @logger.error "QueueProcessor: Invalid command format, missing required fields" end diff --git a/openc3/lib/openc3/models/queue_model.rb b/openc3/lib/openc3/models/queue_model.rb index 1b01a92573..157ed13c32 100644 --- a/openc3/lib/openc3/models/queue_model.rb +++ b/openc3/lib/openc3/models/queue_model.rb @@ -74,7 +74,7 @@ def self.build_command_data(username:, command: nil, target_name: nil, cmd_name: end command_data[:validate] = validate unless validate.nil? command_data[:timeout] = timeout unless timeout.nil? - command_data[:extra] = extra unless extra.nil? + command_data[:extra] = JSON.generate(extra.as_json, allow_nan: true) unless extra.nil? command_data end diff --git a/openc3/python/openc3/microservices/interface_microservice.py b/openc3/python/openc3/microservices/interface_microservice.py index c0fe5a7422..8acc2ae013 100644 --- a/openc3/python/openc3/microservices/interface_microservice.py +++ b/openc3/python/openc3/microservices/interface_microservice.py @@ -323,6 +323,9 @@ def process_cmd(self, topic, msg_id, msg_hash, _redis): command.extra = command.extra or {} if msg_hash.get(b"extra"): command.extra.update(json.loads(msg_hash[b"extra"], cls=JsonDecoder)) + # These fields are populated from COSMOS workflow state below. + command.extra.pop("queue_username", None) + command.extra.pop("approver", None) command.extra["cmd_string"] = msg_hash.get(b"cmd_string", b"").decode() command.extra["username"] = msg_hash.get(b"username", b"").decode() command.extra["interface_name"] = self.interface.name diff --git a/openc3/python/openc3/models/queue_model.py b/openc3/python/openc3/models/queue_model.py index 4e1f78c2a9..36d69c9bca 100644 --- a/openc3/python/openc3/models/queue_model.py +++ b/openc3/python/openc3/models/queue_model.py @@ -15,6 +15,7 @@ from openc3.models.model import Model from openc3.topics.queue_topic import QueueTopic +from openc3.utilities.json import JsonEncoder from openc3.utilities.store import Store @@ -76,7 +77,7 @@ def queue_command( "timestamp": time.time_ns(), } if extra is not None: - command_data["extra"] = extra + command_data["extra"] = json.dumps(extra, cls=JsonEncoder) Store.zadd(f"{scope}:{name}", {json.dumps(command_data): index}) model.notify(kind="command") else: diff --git a/openc3/python/test/microservices/test_interface_microservice.py b/openc3/python/test/microservices/test_interface_microservice.py index 386a3ef532..5b5077fdcb 100644 --- a/openc3/python/test/microservices/test_interface_microservice.py +++ b/openc3/python/test/microservices/test_interface_microservice.py @@ -526,7 +526,14 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): b"hazardous_check": b"TRUE", b"cmd_string": b"cmd('INST ABORT')", b"username": b"test_user", - b"extra": json.dumps({"flow_uuid": "1234-5678", "username": "untrusted"}).encode(), + b"extra": json.dumps( + { + "flow_uuid": "1234-5678", + "username": "untrusted", + "queue_username": "untrusted", + "approver": "untrusted", + } + ).encode(), b"queue_username": b"DEFAULT__MULTI__INST", b"validate": b"TRUE", b"manual": b"FALSE", @@ -541,6 +548,7 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): self.assertEqual(command.extra["flow_uuid"], "1234-5678") self.assertEqual(command.extra["username"], "test_user") self.assertEqual(command.extra.get("queue_username"), "DEFAULT__MULTI__INST") + self.assertNotIn("approver", command.extra) # Minimal msg_hash — only required fields; optional fields use .get() defaults minimal_msg_hash = { diff --git a/openc3/python/test/models/test_queue_model.py b/openc3/python/test/models/test_queue_model.py index ccc11353c7..097e2714af 100644 --- a/openc3/python/test/models/test_queue_model.py +++ b/openc3/python/test/models/test_queue_model.py @@ -15,6 +15,7 @@ from openc3.models.queue_model import QueueError, QueueModel from openc3.topics.queue_topic import QueueTopic +from openc3.utilities.json import JsonDecoder from test.test_helper import mock_redis @@ -109,11 +110,14 @@ def test_queue_command_with_extra_metadata(self, mock_get_model, mock_store): command="CMD", username="user", scope="DEFAULT", - extra={"flow_uuid": "1234-5678"}, + extra={"flow_uuid": "1234-5678", "data": b"\xff"}, ) queued = json.loads(next(iter(mock_store.zadd.call_args.args[1]))) - self.assertEqual(queued["extra"], {"flow_uuid": "1234-5678"}) + self.assertEqual( + json.loads(queued["extra"], cls=JsonDecoder), + {"flow_uuid": "1234-5678", "data": b"\xff"}, + ) @patch("openc3.models.queue_model.Store") @patch("openc3.models.queue_model.QueueModel.get_model") diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index 816c03dec0..5466d70509 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -282,7 +282,10 @@ class ApiTest # queue_username is the original author (shown as "Queued By"), passed # by the queue microservice when a command is released from a queue @api.cmd("INST", "ABORT", queue_username: "DEFAULT__MULTI__INST", - extra: { 'flow_uuid' => '1234-5678', 'username' => 'untrusted' }) + extra: { + 'flow_uuid' => '1234-5678', 'username' => 'untrusted', + 'queue_username' => 'untrusted', 'approver' => 'untrusted' + }) sleep 0.01 im.shutdown @@ -292,6 +295,7 @@ class ApiTest expect(captured.extra['flow_uuid']).to eql('1234-5678') expect(captured.extra['username']).to_not eql('untrusted') expect(captured.extra['queue_username']).to eql("DEFAULT__MULTI__INST") + expect(captured.extra).not_to have_key('approver') end it "handles obfuscated params" do diff --git a/openc3/spec/microservices/queue_microservice_spec.rb b/openc3/spec/microservices/queue_microservice_spec.rb index 917d866710..401cf821b2 100644 --- a/openc3/spec/microservices/queue_microservice_spec.rb +++ b/openc3/spec/microservices/queue_microservice_spec.rb @@ -88,7 +88,7 @@ module OpenC3 let(:command2) { { 'username' => 'test_user', 'value' => 'cmd("TARGET", "COMMAND2", {"PARAM": 2})' } } let(:command3_new_format) do { 'username' => 'test_user', 'target_name' => 'TARGET', 'cmd_name' => 'COMMAND3', - 'cmd_params' => JSON.generate({ 'PARAM' => 3 }), 'extra' => { 'flow_uuid' => '1234-5678' } } + 'cmd_params' => JSON.generate({ 'PARAM' => 3 }), 'extra' => JSON.generate({ 'flow_uuid' => '1234-5678' }) } end let(:command4_new_format) { { 'username' => 'test_user', 'target_name' => 'TARGET', 'cmd_name' => 'COMMAND4' } } diff --git a/openc3/spec/models/queue_model_spec.rb b/openc3/spec/models/queue_model_spec.rb index 7b64cf61bc..06000d82c1 100644 --- a/openc3/spec/models/queue_model_spec.rb +++ b/openc3/spec/models/queue_model_spec.rb @@ -152,8 +152,9 @@ module OpenC3 model.create allow(QueueTopic).to receive(:write_notification) + extra = { "flow_uuid" => "1234-5678", "data" => "\xFF".b } QueueModel.queue_command("TEST", target_name: "INST", cmd_name: "COLLECT", cmd_params: { "TYPE" => "NORMAL" }, - extra: { "flow_uuid" => "1234-5678" }, username: 'test_user', scope: "DEFAULT") + extra: extra, username: 'test_user', scope: "DEFAULT") commands = Store.zrange("DEFAULT:TEST", 0, -1).map { |cmd| JSON.parse(cmd) } expect(commands).to contain_exactly({ @@ -161,7 +162,7 @@ module OpenC3 "target_name" => "INST", "cmd_name" => "COLLECT", "cmd_params" => "{\"TYPE\":\"NORMAL\"}", - "extra" => { "flow_uuid" => "1234-5678" }, + "extra" => JSON.generate(extra.as_json, allow_nan: true), "validate" => true, "timestamp" => anything }) From 55faa650e7ef1be277748c29207af58c5e996779 Mon Sep 17 00:00:00 2001 From: Jonathan Walters Date: Wed, 9 Sep 2026 13:59:52 -0400 Subject: [PATCH 3/7] fix(commands): address metadata review feedback --- docs.openc3.com/docs/guides/scripting-api.md | 4 ++++ .../app/controllers/queues_controller.rb | 10 ++++++---- .../spec/controllers/queues_controller_spec.rb | 7 ++++++- .../lib/openc3/microservices/interface_microservice.rb | 2 ++ .../openc3/microservices/interface_microservice.py | 2 ++ openc3/python/openc3/topics/command_topic.py | 2 +- .../test/microservices/test_interface_microservice.py | 10 +++++++++- openc3/python/test/topics/test_command_topic.py | 6 ++++++ .../spec/microservices/interface_microservice_spec.rb | 5 ++++- 9 files changed, 40 insertions(+), 8 deletions(-) diff --git a/docs.openc3.com/docs/guides/scripting-api.md b/docs.openc3.com/docs/guides/scripting-api.md index d1874bb32f..2ca01dc0e2 100644 --- a/docs.openc3.com/docs/guides/scripting-api.md +++ b/docs.openc3.com/docs/guides/scripting-api.md @@ -842,6 +842,10 @@ cmd("", "", "Param #1 Name" => , "Par | validate | Optional named parameter to enable/disable validation (default is True) | | extra | Optional metadata Hash/dict carried with the command packet to the interface | +The `extra` keys `cmd_string`, `username`, `interface_name`, `queue_username`, `approver`, +`cmd_success`, and `cmd_reason` are reserved for COSMOS audit data. Caller-supplied values for +these keys are discarded or replaced by authoritative values as the command is processed. + diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb index 557283e824..b0c1693c22 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb @@ -270,6 +270,8 @@ def exec_command # Support both new format (target_name, cmd_name, cmd_params) and legacy format (value) validate = command_data.key?('validate') ? command_data['validate'] : true timeout = command_data['timeout'] # Default is nil which means use system default timeout + extra = command_data['extra'] + extra = JSON.parse(extra, allow_nan: true, create_additions: true) if extra.is_a?(String) if command_data['target_name'] && command_data['cmd_name'] # New format: use 3-parameter cmd() method if command_data['cmd_params'] @@ -278,16 +280,16 @@ def exec_command cmd_params = {} end if hazardous - cmd_no_hazardous_check(command_data['target_name'], command_data['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], scope: params[:scope], token: token) + cmd_no_hazardous_check(command_data['target_name'], command_data['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], extra: extra, scope: params[:scope], token: token) else - cmd(command_data['target_name'], command_data['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], scope: params[:scope], token: token) + cmd(command_data['target_name'], command_data['cmd_name'], cmd_params, queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], extra: extra, scope: params[:scope], token: token) end elsif command_data['value'] # Legacy format: use single string parameter if hazardous - cmd_no_hazardous_check(command_data['value'], queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], scope: params[:scope], token: token) + cmd_no_hazardous_check(command_data['value'], queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], extra: extra, scope: params[:scope], token: token) else - cmd(command_data['value'], queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], scope: params[:scope], token: token) + cmd(command_data['value'], queue: false, validate: validate, timeout: timeout, queue_username: command_data['username'], extra: extra, scope: params[:scope], token: token) end else log_error("Invalid command format in queue: #{command_data}") diff --git a/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb b/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb index 4a1b7bd1d5..32a3e8fe84 100644 --- a/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb +++ b/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb @@ -550,6 +550,7 @@ def generate_queue_hash command_data = { "username" => "user1", "value" => "TEST COMMAND", + "extra" => JSON.generate({ "flow_uuid" => "1234-5678", "data" => "\xFF".b }.as_json, allow_nan: true), "timestamp" => 1000, "id" => 1.0 } @@ -561,6 +562,7 @@ def generate_queue_hash validate: true, timeout: nil, queue_username: "user1", + extra: { "flow_uuid" => "1234-5678", "data" => "\xFF".b }, scope: "DEFAULT", token: anything } @@ -591,6 +593,7 @@ def generate_queue_hash validate: true, timeout: nil, queue_username: "user2", + extra: nil, scope: "DEFAULT", token: anything } @@ -658,6 +661,7 @@ def generate_queue_hash validate: true, timeout: nil, queue_username: "user3", + extra: nil, scope: "DEFAULT", token: anything } @@ -687,6 +691,7 @@ def generate_queue_hash validate: true, timeout: 0, queue_username: "user4", + extra: nil, scope: "DEFAULT", token: anything } @@ -779,4 +784,4 @@ def generate_queue_hash ) end end -end \ No newline at end of file +end diff --git a/openc3/lib/openc3/microservices/interface_microservice.rb b/openc3/lib/openc3/microservices/interface_microservice.rb index e28e1bfb7f..5a3a09e463 100644 --- a/openc3/lib/openc3/microservices/interface_microservice.rb +++ b/openc3/lib/openc3/microservices/interface_microservice.rb @@ -290,6 +290,8 @@ def run # These fields are populated from COSMOS workflow state below. command.extra.delete('queue_username') command.extra.delete('approver') + command.extra.delete('cmd_success') + command.extra.delete('cmd_reason') command.extra['cmd_string'] = msg_hash['cmd_string'] command.extra['username'] = msg_hash['username'] command.extra['interface_name'] = @interface.name diff --git a/openc3/python/openc3/microservices/interface_microservice.py b/openc3/python/openc3/microservices/interface_microservice.py index 8acc2ae013..30f0fba144 100644 --- a/openc3/python/openc3/microservices/interface_microservice.py +++ b/openc3/python/openc3/microservices/interface_microservice.py @@ -326,6 +326,8 @@ def process_cmd(self, topic, msg_id, msg_hash, _redis): # These fields are populated from COSMOS workflow state below. command.extra.pop("queue_username", None) command.extra.pop("approver", None) + command.extra.pop("cmd_success", None) + command.extra.pop("cmd_reason", None) command.extra["cmd_string"] = msg_hash.get(b"cmd_string", b"").decode() command.extra["username"] = msg_hash.get(b"username", b"").decode() command.extra["interface_name"] = self.interface.name diff --git a/openc3/python/openc3/topics/command_topic.py b/openc3/python/openc3/topics/command_topic.py index 8fa32a403c..05ff0b6d27 100644 --- a/openc3/python/openc3/topics/command_topic.py +++ b/openc3/python/openc3/topics/command_topic.py @@ -36,7 +36,7 @@ def write_packet(cls, packet, scope): "buffer": bytes(packet.buffer_no_copy()), } if packet.extra: - msg_hash["extra"] = json.dumps(packet.extra) + msg_hash["extra"] = json.dumps(packet.extra, cls=JsonEncoder) db_shard = Store.db_shard_for_target(packet.target_name, scope=scope) EphemeralStoreQueued.instance(db_shard=db_shard).write_topic(topic, msg_hash) diff --git a/openc3/python/test/microservices/test_interface_microservice.py b/openc3/python/test/microservices/test_interface_microservice.py index 5b5077fdcb..726146f781 100644 --- a/openc3/python/test/microservices/test_interface_microservice.py +++ b/openc3/python/test/microservices/test_interface_microservice.py @@ -28,6 +28,7 @@ from openc3.topics.interface_topic import InterfaceTopic from openc3.topics.telemetry_decom_topic import TelemetryDecomTopic from openc3.topics.topic import Topic +from openc3.utilities.json import JsonEncoder from openc3.utilities.store_queued import EphemeralStoreQueued, StoreQueued from openc3.utilities.time import from_nsec_from_epoch from test.test_helper import * @@ -532,7 +533,11 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): "username": "untrusted", "queue_username": "untrusted", "approver": "untrusted", - } + "cmd_success": False, + "cmd_reason": "untrusted", + "data": b"\xff", + }, + cls=JsonEncoder, ).encode(), b"queue_username": b"DEFAULT__MULTI__INST", b"validate": b"TRUE", @@ -549,6 +554,9 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): self.assertEqual(command.extra["username"], "test_user") self.assertEqual(command.extra.get("queue_username"), "DEFAULT__MULTI__INST") self.assertNotIn("approver", command.extra) + self.assertNotIn("cmd_success", command.extra) + self.assertNotIn("cmd_reason", command.extra) + self.assertEqual(command.extra["data"], b"\xff") # Minimal msg_hash — only required fields; optional fields use .get() defaults minimal_msg_hash = { diff --git a/openc3/python/test/topics/test_command_topic.py b/openc3/python/test/topics/test_command_topic.py index 2f027d7700..6798bd74ac 100644 --- a/openc3/python/test/topics/test_command_topic.py +++ b/openc3/python/test/topics/test_command_topic.py @@ -15,6 +15,7 @@ from unittest.mock import MagicMock, patch from openc3.topics.command_topic import CommandTopic +from openc3.utilities.json import JsonDecoder from test.test_helper import mock_redis @@ -71,6 +72,11 @@ def test_includes_extra_when_set(self): self.assertIn("extra", self.captured["msg_hash"]) self.assertEqual(json.loads(self.captured["msg_hash"]["extra"]), extra) + def test_encodes_binary_extra(self): + extra = {"data": b"\xff"} + CommandTopic.write_packet(self._make_packet(extra=extra), scope="DEFAULT") + self.assertEqual(json.loads(self.captured["msg_hash"]["extra"], cls=JsonDecoder), extra) + def test_omits_extra_when_none(self): CommandTopic.write_packet(self._make_packet(extra=None), scope="DEFAULT") self.assertNotIn("extra", self.captured["msg_hash"]) diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index 5466d70509..1856db1892 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -284,7 +284,8 @@ class ApiTest @api.cmd("INST", "ABORT", queue_username: "DEFAULT__MULTI__INST", extra: { 'flow_uuid' => '1234-5678', 'username' => 'untrusted', - 'queue_username' => 'untrusted', 'approver' => 'untrusted' + 'queue_username' => 'untrusted', 'approver' => 'untrusted', + 'cmd_success' => false, 'cmd_reason' => 'untrusted' }) sleep 0.01 im.shutdown @@ -296,6 +297,8 @@ class ApiTest expect(captured.extra['username']).to_not eql('untrusted') expect(captured.extra['queue_username']).to eql("DEFAULT__MULTI__INST") expect(captured.extra).not_to have_key('approver') + expect(captured.extra).not_to have_key('cmd_success') + expect(captured.extra).not_to have_key('cmd_reason') end it "handles obfuscated params" do From 85296d9126a06ac9420c4a91f93c7276613fb217 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 9 Sep 2026 17:54:45 -0600 Subject: [PATCH 4/7] fix(commands): keep accessor extra authoritative over caller metadata Caller extra was merged over command.extra, so it overwrote the keys the packet's accessor had just written during build_cmd. For an HTTP target that meant HTTP_PATH, HTTP_METHOD, HTTP_HEADERS and HTTP_QUERIES became caller controlled, letting anyone with cmd permission on the target redirect the request the interface makes on their behalf. Merge the other way so packet derived values win. Keys the accessor did not set still come through, so the feature is unchanged. Co-Authored-By: Claude Opus 5 (1M context) --- docs.openc3.com/docs/guides/scripting-api.md | 10 ++- .../microservices/interface_microservice.rb | 9 +- .../microservices/interface_microservice.py | 10 ++- .../test_interface_microservice.py | 88 +++++++++++++++++++ .../interface_microservice_spec.rb | 35 ++++++++ 5 files changed, 149 insertions(+), 3 deletions(-) diff --git a/docs.openc3.com/docs/guides/scripting-api.md b/docs.openc3.com/docs/guides/scripting-api.md index 2ca01dc0e2..3c666fdcac 100644 --- a/docs.openc3.com/docs/guides/scripting-api.md +++ b/docs.openc3.com/docs/guides/scripting-api.md @@ -840,12 +840,20 @@ cmd("", "", "Param #1 Name" => , "Par | timeout | Optional named parameter to change the default timeout value of 5 seconds | | log_message | Optional named parameter to prevent logging of the command | | validate | Optional named parameter to enable/disable validation (default is True) | -| extra | Optional metadata Hash/dict carried with the command packet to the interface | +| extra | Optional metadata Hash/dict carried with the command packet to the interface | The `extra` keys `cmd_string`, `username`, `interface_name`, `queue_username`, `approver`, `cmd_success`, and `cmd_reason` are reserved for COSMOS audit data. Caller-supplied values for these keys are discarded or replaced by authoritative values as the command is processed. +Keys that a packet's accessor writes into `extra` are also reserved. HTTP targets are the +common case: `HTTP_PATH`, `HTTP_METHOD`, `HTTP_STATUS`, `HTTP_PACKET`, `HTTP_ERROR_PACKET`, +`HTTP_HEADERS` and `HTTP_QUERIES` come from the command definition and always win over +caller-supplied values, so `extra` cannot redirect the request an interface makes. + +`extra` is written to the command log, the command topics and the queue exactly as given. +It is not obfuscated, so don't put secrets in it. + diff --git a/openc3/lib/openc3/microservices/interface_microservice.rb b/openc3/lib/openc3/microservices/interface_microservice.rb index 5a3a09e463..878fd14588 100644 --- a/openc3/lib/openc3/microservices/interface_microservice.rb +++ b/openc3/lib/openc3/microservices/interface_microservice.rb @@ -285,7 +285,14 @@ def run command.extra ||= {} if msg_hash['extra'] - command.extra.merge!(JSON.parse(msg_hash['extra'], allow_nan: true, create_additions: true)) + # Caller metadata is the base layer. Values already in command.extra were + # written by the packet's accessor during build_cmd (e.g. HttpAccessor sets + # HTTP_PATH, HTTP_METHOD, HTTP_HEADERS, HTTP_QUERIES from the command + # definition) and are authoritative, so they overwrite caller values rather + # than the other way around. Otherwise a caller could redirect the request + # an interface makes on their behalf. + caller_extra = JSON.parse(msg_hash['extra'], allow_nan: true, create_additions: true) + command.extra = caller_extra.merge(command.extra) end # These fields are populated from COSMOS workflow state below. command.extra.delete('queue_username') diff --git a/openc3/python/openc3/microservices/interface_microservice.py b/openc3/python/openc3/microservices/interface_microservice.py index 30f0fba144..0a97203300 100644 --- a/openc3/python/openc3/microservices/interface_microservice.py +++ b/openc3/python/openc3/microservices/interface_microservice.py @@ -322,7 +322,15 @@ def process_cmd(self, topic, msg_id, msg_hash, _redis): command.extra = command.extra or {} if msg_hash.get(b"extra"): - command.extra.update(json.loads(msg_hash[b"extra"], cls=JsonDecoder)) + # Caller metadata is the base layer. Values already in command.extra were + # written by the packet's accessor during build_cmd (e.g. HttpAccessor sets + # HTTP_PATH, HTTP_METHOD, HTTP_HEADERS, HTTP_QUERIES from the command + # definition) and are authoritative, so they overwrite caller values rather + # than the other way around. Otherwise a caller could redirect the request + # an interface makes on their behalf. + caller_extra = json.loads(msg_hash[b"extra"], cls=JsonDecoder) + caller_extra.update(command.extra) + command.extra = caller_extra # These fields are populated from COSMOS workflow state below. command.extra.pop("queue_username", None) command.extra.pop("approver", None) diff --git a/openc3/python/test/microservices/test_interface_microservice.py b/openc3/python/test/microservices/test_interface_microservice.py index 726146f781..d7e0cfc82c 100644 --- a/openc3/python/test/microservices/test_interface_microservice.py +++ b/openc3/python/test/microservices/test_interface_microservice.py @@ -10,6 +10,7 @@ # if purchased from OpenC3, Inc. import json +import math import threading import time import unittest @@ -580,6 +581,93 @@ def test_process_cmd_with_all_fields_and_missing_optional_fields(self): result = handler.process_cmd(topic, msg_id, full_msg_hash, None) self.assertIsNone(result) + def test_process_cmd_decodes_ruby_encoded_special_floats_in_extra(self): + """A command queued from Ruby (or released by the Ruby queue microservice) has + its extra encoded by Ruby's Float#as_json, which writes non-finite floats as + {"json_class": "Float", "raw": "NaN"|"Infinity"|"-Infinity"}. Those must arrive + at a Python interface as floats, not as dicts.""" + im = InterfaceMicroservice("DEFAULT__INTERFACE__INST_INT") + thread = threading.Thread(target=im.run) + thread.start() + self.addCleanup(thread.join, 5) + self.addCleanup(im.shutdown) + time.sleep(0.1) + + handler = im.handler_thread + # Byte for byte what Ruby's JSON.generate(extra.as_json, allow_nan: true) emits + ruby_extra = ( + b'{"nan":{"json_class":"Float","raw":"NaN"},' + b'"inf":{"json_class":"Float","raw":"Infinity"},' + b'"ninf":{"json_class":"Float","raw":"-Infinity"},' + b'"normal":1.5}' + ) + msg_hash = { + b"target_name": b"INST", + b"cmd_name": b"ABORT", + b"cmd_params": json.dumps({}).encode(), + b"cmd_string": b"cmd('INST ABORT')", + b"username": b"test_user", + b"extra": ruby_extra, + } + with patch("openc3.microservices.interface_microservice.CommandDecomTopic.write_packet") as mock_write: + result = handler.process_cmd("{DEFAULT__CMD}TARGET__INST", "1-0", msg_hash, None) + self.assertEqual(result, "SUCCESS") + + command = mock_write.call_args[0][0] + self.assertTrue(math.isnan(command.extra["nan"])) + self.assertEqual(command.extra["inf"], float("inf")) + self.assertEqual(command.extra["ninf"], float("-inf")) + self.assertEqual(command.extra["normal"], 1.5) + + def test_process_cmd_does_not_let_caller_metadata_override_accessor_extra(self): + """Accessors write into packet.extra while build_cmd sets the command + parameters. HttpAccessor puts HTTP_PATH / HTTP_METHOD / HTTP_HEADERS / + HTTP_QUERIES there and HttpClientInterface builds the outgoing request from + them, so caller metadata must never win over these.""" + im = InterfaceMicroservice("DEFAULT__INTERFACE__INST_INT") + thread = threading.Thread(target=im.run) + thread.start() + self.addCleanup(thread.join, 5) + self.addCleanup(im.shutdown) + time.sleep(0.1) + + handler = im.handler_thread + original_build_cmd = System.commands.build_cmd + + def build_cmd_with_accessor_extra(*args, **kwargs): + command = original_build_cmd(*args, **kwargs) + command.extra = {"HTTP_PATH": "/defined", "HTTP_METHOD": "get"} + return command + + msg_hash = { + b"target_name": b"INST", + b"cmd_name": b"ABORT", + b"cmd_params": json.dumps({}).encode(), + b"cmd_string": b"cmd('INST ABORT')", + b"username": b"test_user", + b"extra": json.dumps( + { + "HTTP_PATH": "/attacker", + "HTTP_METHOD": "delete", + "HTTP_HEADERS": {"authorization": "stolen"}, + "flow_uuid": "1234-5678", + } + ).encode(), + } + with ( + patch.object(System.commands, "build_cmd", side_effect=build_cmd_with_accessor_extra), + patch("openc3.microservices.interface_microservice.CommandDecomTopic.write_packet") as mock_write, + ): + result = handler.process_cmd("{DEFAULT__CMD}TARGET__INST", "1-0", msg_hash, None) + self.assertEqual(result, "SUCCESS") + + command = mock_write.call_args[0][0] + self.assertEqual(command.extra["HTTP_PATH"], "/defined") + self.assertEqual(command.extra["HTTP_METHOD"], "get") + # Keys the accessor didn't set still come through so the feature works + self.assertEqual(command.extra["HTTP_HEADERS"], {"authorization": "stolen"}) + self.assertEqual(command.extra["flow_uuid"], "1234-5678") + def test_process_cmd_supports_interface_directives(self): """Directive messages on the CMD}INTERFACE topic: interface_details and target_control (enable/disable and the error path).""" diff --git a/openc3/spec/microservices/interface_microservice_spec.rb b/openc3/spec/microservices/interface_microservice_spec.rb index 1856db1892..533c36a7b7 100644 --- a/openc3/spec/microservices/interface_microservice_spec.rb +++ b/openc3/spec/microservices/interface_microservice_spec.rb @@ -301,6 +301,41 @@ class ApiTest expect(captured.extra).not_to have_key('cmd_reason') end + it "does not let caller metadata override accessor populated extra" do + im = InterfaceMicroservice.new("DEFAULT__INTERFACE__INST_INT") + + # Accessors write into packet.extra while build_cmd sets the command + # parameters. HttpAccessor puts HTTP_PATH / HTTP_METHOD / HTTP_HEADERS / + # HTTP_QUERIES there and HttpClientInterface builds the outgoing request + # from them, so caller metadata must never win over these. + allow(System.commands).to receive(:build_cmd).and_wrap_original do |original, *args| + command = original.call(*args) + command.extra = { 'HTTP_PATH' => '/defined', 'HTTP_METHOD' => 'get' } + command + end + + captured = nil + allow(CommandDecomTopic).to receive(:write_packet) do |command, _scope| + captured = command + end + Thread.new { im.run } + sleep 0.01 + + @api.cmd("INST", "ABORT", extra: { + 'HTTP_PATH' => '/attacker', 'HTTP_METHOD' => 'delete', + 'HTTP_HEADERS' => { 'authorization' => 'stolen' }, 'flow_uuid' => '1234-5678' + }) + sleep 0.01 + im.shutdown + + expect(captured).to_not be_nil + expect(captured.extra['HTTP_PATH']).to eql('/defined') + expect(captured.extra['HTTP_METHOD']).to eql('get') + # Keys the accessor didn't set still come through so the feature works + expect(captured.extra['HTTP_HEADERS']).to eql({ 'authorization' => 'stolen' }) + expect(captured.extra['flow_uuid']).to eql('1234-5678') + end + it "handles obfuscated params" do im = InterfaceMicroservice.new("DEFAULT__INTERFACE__INST_INT") all = InterfaceStatusModel.all(scope: "DEFAULT") From 976ff0775223c0d66d4db239181f01ef4e366f64 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 9 Sep 2026 17:54:52 -0600 Subject: [PATCH 5/7] fix(queues): preserve command extra through insert and update QueueModel gained an extra keyword but the controller never passed one, so queueing or editing a command through the API dropped the metadata attached by cmd(extra: ...). Pass extra from params on insert_command and update_command, rejecting a non-Hash with 400. update_command carries the existing entry's extra forward when the request omits it, since the queue edit APIs have no way to express it. Co-Authored-By: Claude Opus 5 (1M context) --- .../app/controllers/queues_controller.rb | 27 ++++++-- .../controllers/queues_controller_spec.rb | 64 +++++++++++++++++++ openc3/lib/openc3/models/queue_model.rb | 9 ++- openc3/spec/models/queue_model_spec.rb | 37 +++++++++++ 4 files changed, 132 insertions(+), 5 deletions(-) diff --git a/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb b/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb index b0c1693c22..d43a959638 100644 --- a/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb +++ b/openc3-cosmos-cmd-tlm-api/app/controllers/queues_controller.rb @@ -164,15 +164,18 @@ def insert_command end end id = params[:id]&.to_f + extra = extra_param() # If id is nil this means insert at the end if target_name && cmd_name model.insert_command(id: id, username: username(), target_name: target_name, cmd_name: cmd_name, - cmd_params: params[:cmd_params], validate: params[:validate], timeout: params[:timeout]) + cmd_params: params[:cmd_params], extra: extra, validate: params[:validate], timeout: params[:timeout]) else - model.insert_command(id: id, username: username(), command: command, + model.insert_command(id: id, username: username(), command: command, extra: extra, validate: params[:validate], timeout: params[:timeout]) end render json: { status: 'success', message: 'Command added to queue' } + rescue ArgumentError => e + render json: { status: 'error', message: e.message, type: e.class.to_s }, status: :bad_request rescue StandardError => e log_error(e) render json: { status: 'error', message: e.message, type: e.class.to_s, backtrace: e.backtrace }, status: :internal_server_error @@ -237,13 +240,16 @@ def update_command # validate should be true or false, default to true if not given validate = params[:validate].nil? ? true : params[:validate] # timeout can be nil which means use system default timeout + extra = extra_param() if target_name && cmd_name model.update_command(id: id, username: username(), target_name: target_name, cmd_name: cmd_name, - cmd_params: params[:cmd_params], validate: validate, timeout: params[:timeout]) + cmd_params: params[:cmd_params], extra: extra, validate: validate, timeout: params[:timeout]) else - model.update_command(id: id, username: username(), command: command, validate: validate, timeout: params[:timeout]) + model.update_command(id: id, username: username(), command: command, extra: extra, validate: validate, timeout: params[:timeout]) end render json: { status: 'success', message: 'Command updated' } + rescue ArgumentError => e + render json: { status: 'error', message: e.message, type: e.class.to_s }, status: :bad_request rescue OpenC3::QueueError => e log_error(e) render json: { status: 'error', message: e.message, type: e.class.to_s }, status: :bad_request @@ -332,6 +338,19 @@ def destroy private + # Caller metadata attached to a queued command. Rails wraps nested params in + # ActionController::Parameters so convert back to a plain Hash before it is + # JSON encoded into the queue entry. + def extra_param + extra = params[:extra] + return nil if extra.nil? + + extra = extra.to_unsafe_h if extra.respond_to?(:to_unsafe_h) + raise ArgumentError, "Invalid extra parameter: #{extra}. Must be a Hash." unless extra.is_a?(Hash) + + extra + end + def change_state(params, state) return unless authorization('cmd') begin diff --git a/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb b/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb index 32a3e8fe84..10dacc5dc8 100644 --- a/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb +++ b/openc3-cosmos-cmd-tlm-api/spec/controllers/queues_controller_spec.rb @@ -281,6 +281,7 @@ def generate_queue_hash id: nil, username: "anonymous", command: "TEST COMMAND", + extra: nil, validate: nil, timeout: nil ) @@ -301,11 +302,42 @@ def generate_queue_hash id: id.to_f, username: "anonymous", command: "TEST COMMAND", + extra: nil, validate: nil, timeout: nil ) end + it "passes extra metadata through to the model" do + queue_model = double("QueueModel") + allow(OpenC3::QueueModel).to receive(:get_model).and_return(queue_model) + allow(queue_model).to receive(:insert_command) + + post :insert_command, params: {name: "QUEUE1", command: "TEST COMMAND", extra: {flow_uuid: "1234-5678"}, scope: "DEFAULT"} + expect(response).to have_http_status(:ok) + expect(queue_model).to have_received(:insert_command).with( + id: nil, + username: "anonymous", + command: "TEST COMMAND", + extra: {"flow_uuid" => "1234-5678"}, + validate: nil, + timeout: nil + ) + end + + it "returns 400 when extra is not a Hash" do + queue_model = double("QueueModel") + allow(OpenC3::QueueModel).to receive(:get_model).and_return(queue_model) + allow(queue_model).to receive(:insert_command) + + post :insert_command, params: {name: "QUEUE1", command: "TEST COMMAND", extra: "invalid", scope: "DEFAULT"} + expect(response).to have_http_status(400) + json = JSON.parse(response.body, allow_nan: true, create_additions: true) + expect(json["status"]).to eql("error") + expect(json["message"]).to eql("Invalid extra parameter: invalid. Must be a Hash.") + expect(queue_model).to_not have_received(:insert_command) + end + it "returns 404 when the queue is not found" do allow(OpenC3::QueueModel).to receive(:get_model).and_return(nil) @@ -440,11 +472,43 @@ def generate_queue_hash id: id, username: "anonymous", command: "UPDATED COMMAND", + extra: nil, validate: true, # Even when not provided, validate should default to true timeout: nil ) end + it "passes extra metadata through to the model" do + queue_model = double("QueueModel") + allow(OpenC3::QueueModel).to receive(:get_model).and_return(queue_model) + allow(queue_model).to receive(:update_command) + + post :update_command, params: {name: "QUEUE1", command: "UPDATED COMMAND", id: "1.0", + extra: {flow_uuid: "1234-5678"}, scope: "DEFAULT"} + expect(response).to have_http_status(:ok) + expect(queue_model).to have_received(:update_command).with( + id: "1.0", + username: "anonymous", + command: "UPDATED COMMAND", + extra: {"flow_uuid" => "1234-5678"}, + validate: true, + timeout: nil + ) + end + + it "returns 400 when extra is not a Hash" do + queue_model = double("QueueModel") + allow(OpenC3::QueueModel).to receive(:get_model).and_return(queue_model) + allow(queue_model).to receive(:update_command) + + post :update_command, params: {name: "QUEUE1", command: "UPDATED COMMAND", id: "1.0", extra: "invalid", scope: "DEFAULT"} + expect(response).to have_http_status(400) + json = JSON.parse(response.body, allow_nan: true, create_additions: true) + expect(json["status"]).to eql("error") + expect(json["message"]).to eql("Invalid extra parameter: invalid. Must be a Hash.") + expect(queue_model).to_not have_received(:update_command) + end + it "returns 404 when the queue is not found" do allow(OpenC3::QueueModel).to receive(:get_model).and_return(nil) diff --git a/openc3/lib/openc3/models/queue_model.rb b/openc3/lib/openc3/models/queue_model.rb index 157ed13c32..cdd1128dc0 100644 --- a/openc3/lib/openc3/models/queue_model.rb +++ b/openc3/lib/openc3/models/queue_model.rb @@ -74,7 +74,10 @@ def self.build_command_data(username:, command: nil, target_name: nil, cmd_name: end command_data[:validate] = validate unless validate.nil? command_data[:timeout] = timeout unless timeout.nil? - command_data[:extra] = JSON.generate(extra.as_json, allow_nan: true) unless extra.nil? + unless extra.nil? + # extra is already a JSON string when carried forward from an existing queue entry + command_data[:extra] = extra.is_a?(String) ? extra : JSON.generate(extra.as_json, allow_nan: true) + end command_data end @@ -146,6 +149,10 @@ def update_command(id:, username:, command: nil, target_name: nil, cmd_name: nil raise QueueError, "No command found at id #{id} in queue '#{@name}'" end + # Carry forward the caller metadata attached by cmd(extra: ...) when the update + # doesn't supply its own. Editing a queued command shouldn't silently drop it. + extra = JSON.parse(existing[0])['extra'] if extra.nil? + Store.zremrangebyscore("#{@scope}:#{@name}", id, id) command_data = self.class.build_command_data(username: username, command: command, target_name: target_name, cmd_name: cmd_name, cmd_params: cmd_params, extra: extra, validate: validate, timeout: timeout) diff --git a/openc3/spec/models/queue_model_spec.rb b/openc3/spec/models/queue_model_spec.rb index 06000d82c1..1be794c1e8 100644 --- a/openc3/spec/models/queue_model_spec.rb +++ b/openc3/spec/models/queue_model_spec.rb @@ -421,6 +421,17 @@ module OpenC3 decoded = result["DATA"]["raw"].pack('C*') expect(decoded).to eq(binary_data) end + + it "stores extra when inserting" do + allow(QueueTopic).to receive(:write_notification) + model = QueueModel.new(name: "TEST", scope: "DEFAULT") + + extra = { "flow_uuid" => "1234-5678", "data" => "\xFF".b } + model.insert_command(id: 1, username: "test_user", command: "TGT CMD", extra: extra) + + commands = Store.zrange("DEFAULT:TEST", 0, -1).map { |cmd| JSON.parse(cmd) } + expect(JSON.parse(commands[0]["extra"], allow_nan: true, create_additions: true)).to eql(extra) + end end describe "update_command" do @@ -489,6 +500,32 @@ module OpenC3 model.update_command(id: 1.0, command: "TGT CMD2", username: "user2") }.to raise_error(QueueError, "Queue 'TEST' is disabled. Command at id 1.0 not updated.") end + + it "carries forward existing extra when the update does not supply it" do + allow(QueueTopic).to receive(:write_notification) + model = QueueModel.new(name: "TEST", scope: "DEFAULT") + + extra = { "flow_uuid" => "1234-5678", "data" => "\xFF".b } + model.insert_command(id: 1.0, username: "user1", command: "TGT CMD1", extra: extra) + # The queue edit APIs have no way to express extra, so an update that omits + # it must not silently drop the metadata attached by cmd(extra: ...) + model.update_command(id: 1.0, command: "TGT CMD2", username: "user2") + + commands = Store.zrange("DEFAULT:TEST", 0, -1).map { |cmd| JSON.parse(cmd) } + expect(commands[0]["value"]).to eq("TGT CMD2") + expect(JSON.parse(commands[0]["extra"], allow_nan: true, create_additions: true)).to eql(extra) + end + + it "replaces existing extra when the update supplies it" do + allow(QueueTopic).to receive(:write_notification) + model = QueueModel.new(name: "TEST", scope: "DEFAULT") + + model.insert_command(id: 1.0, username: "user1", command: "TGT CMD1", extra: { "flow_uuid" => "old" }) + model.update_command(id: 1.0, command: "TGT CMD1", username: "user2", extra: { "flow_uuid" => "new" }) + + commands = Store.zrange("DEFAULT:TEST", 0, -1).map { |cmd| JSON.parse(cmd) } + expect(JSON.parse(commands[0]["extra"])).to eql({ "flow_uuid" => "new" }) + end end describe "list" do From 63bc4a75bff7608161b37976a7f79a7609b2d066 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 9 Sep 2026 17:55:02 -0600 Subject: [PATCH 6/7] fix(logs): encode packet extra as bytes when framing and logging cmd(extra: ...) puts caller controlled values into packet.extra, which exposed two problems on the write path. JSON.generate returns UTF-8. Appending that to the binary log entry or preidentified frame raises Encoding::CompatibilityError once the buffer holds a byte >= 0x80, which the packet time nearly always does, so the packet was silently dropped from the log. Force the JSON to binary. The extra length field also counted characters instead of bytes, which would declare a short length and desync every entry after it. On the Python side PreidentifiedProtocol used plain json.dumps, raising "Object of type bytes is not JSON serializable" for the binary extra values COSMOS supports elsewhere. Use JsonEncoder like the rest of the codebase. Co-Authored-By: Claude Opus 5 (1M context) --- .../protocols/preidentified_protocol.rb | 8 +++-- openc3/lib/openc3/logs/packet_log_writer.rb | 11 ++++-- .../protocols/preidentified_protocol.py | 12 +++++-- .../protocols/test_preidentified_protocol.py | 20 +++++++++++ .../protocols/preidentified_protocol_spec.rb | 20 +++++++++++ openc3/spec/logs/packet_log_writer_spec.rb | 35 +++++++++++++++++++ 6 files changed, 98 insertions(+), 8 deletions(-) diff --git a/openc3/lib/openc3/interfaces/protocols/preidentified_protocol.rb b/openc3/lib/openc3/interfaces/protocols/preidentified_protocol.rb index e57458ee13..df36120438 100644 --- a/openc3/lib/openc3/interfaces/protocols/preidentified_protocol.rb +++ b/openc3/lib/openc3/interfaces/protocols/preidentified_protocol.rb @@ -65,7 +65,9 @@ def write_packet(packet) @write_extra = nil if packet.extra @write_flags |= COSMOS4_EXTRA_FLAG_MASK - @write_extra = packet.extra.as_json().to_json(allow_nan: true) + # Force binary so appending it can't promote data_to_send to UTF-8, which + # then raises Encoding::CompatibilityError on the binary fields that follow + @write_extra = packet.extra.as_json().to_json(allow_nan: true).b end return packet end @@ -76,7 +78,9 @@ def write_data(data, extra = nil) data_to_send << @sync_pattern if @sync_pattern data_to_send << @write_flags if @write_extra - data_to_send << [@write_extra.length].pack('N') + # Length field must count bytes, not characters, or a non-ASCII value in + # extra desyncs the receiver + data_to_send << [@write_extra.bytesize].pack('N') data_to_send << @write_extra end data_to_send << @write_time_seconds diff --git a/openc3/lib/openc3/logs/packet_log_writer.rb b/openc3/lib/openc3/logs/packet_log_writer.rb index 3bbc9da002..6296f3b01f 100644 --- a/openc3/lib/openc3/logs/packet_log_writer.rb +++ b/openc3/lib/openc3/logs/packet_log_writer.rb @@ -313,15 +313,20 @@ def write_entry(entry_type, cmd_or_tlm, target_name, packet_name, time_nsec_sinc if @data_format == :CBOR extra_encoded = extra.as_json.to_cbor else - extra_encoded = JSON.generate(extra.as_json, allow_nan: true) + # Force binary: JSON.generate returns UTF-8 and appending that to the + # binary entry raises Encoding::CompatibilityError once the entry holds + # a byte >= 0x80, which silently drops the packet from the log + extra_encoded = JSON.generate(extra.as_json, allow_nan: true).b end - length += extra_encoded.length + # Count bytes, not characters, so a non-ASCII value in extra can't produce + # a short entry length + length += extra_encoded.bytesize end length += OPENC3_PACKET_SECONDARY_FIXED_SIZE + data.length @entry.clear @entry << [length, flags, packet_index, time_nsec_since_epoch].pack(OPENC3_PACKET_PACK_DIRECTIVE) @entry << [received_time_nsec_since_epoch].pack(OPENC3_RECEIVED_TIME_PACK_DIRECTIVE) if received_time_nsec_since_epoch - @entry << [extra_encoded.length].pack(OPENC3_EXTRA_LENGTH_PACK_DIRECTIVE) << extra_encoded if extra_encoded + @entry << [extra_encoded.bytesize].pack(OPENC3_EXTRA_LENGTH_PACK_DIRECTIVE) << extra_encoded if extra_encoded @entry << data.force_encoding('ASCII-8BIT') @first_time = time_nsec_since_epoch if !@first_time or time_nsec_since_epoch < @first_time @last_time = time_nsec_since_epoch if !@last_time or time_nsec_since_epoch > @last_time diff --git a/openc3/python/openc3/interfaces/protocols/preidentified_protocol.py b/openc3/python/openc3/interfaces/protocols/preidentified_protocol.py index ab14aaa56a..c165b59ae6 100644 --- a/openc3/python/openc3/interfaces/protocols/preidentified_protocol.py +++ b/openc3/python/openc3/interfaces/protocols/preidentified_protocol.py @@ -17,6 +17,7 @@ from openc3.config.config_parser import ConfigParser from openc3.interfaces.protocols.burst_protocol import BurstProtocol from openc3.utilities.extract import convert_to_value +from openc3.utilities.json import JsonEncoder # Delineates packets using the OpenC3 preidentification system @@ -68,7 +69,9 @@ def write_packet(self, packet): self.write_extra = None if packet.extra: self.write_flags |= PreidentifiedProtocol.COSMOS4_EXTRA_FLAG_MASK - self.write_extra = json.dumps(packet.extra) + # JsonEncoder so binary and other COSMOS types survive, matching the Ruby + # side which encodes via as_json + self.write_extra = json.dumps(packet.extra, cls=JsonEncoder) return packet def write_data(self, data, extra=None): @@ -78,8 +81,11 @@ def write_data(self, data, extra=None): data_to_send += self.sync_pattern data_to_send += self.write_flags.to_bytes(1, byteorder="big") if self.write_extra: - data_to_send += struct.pack(">I", len(self.write_extra)) - data_to_send += bytes(self.write_extra, "ascii") + # Length field must count bytes, not characters, or a non-ASCII value in + # extra desyncs the receiver + write_extra = self.write_extra.encode("utf-8") + data_to_send += struct.pack(">I", len(write_extra)) + data_to_send += write_extra data_to_send += self.write_time_seconds data_to_send += self.write_time_microseconds data_to_send += struct.pack(">B", len(self.write_target_name)) diff --git a/openc3/python/test/interfaces/protocols/test_preidentified_protocol.py b/openc3/python/test/interfaces/protocols/test_preidentified_protocol.py index 22134bc305..ebf8aaed0b 100644 --- a/openc3/python/test/interfaces/protocols/test_preidentified_protocol.py +++ b/openc3/python/test/interfaces/protocols/test_preidentified_protocol.py @@ -19,6 +19,7 @@ from openc3.interfaces.protocols.preidentified_protocol import PreidentifiedProtocol from openc3.interfaces.stream_interface import StreamInterface from openc3.streams.stream import Stream +from openc3.utilities.json import JsonDecoder, JsonEncoder from test.test_helper import * @@ -144,6 +145,25 @@ def test_write_creates_a_packet_header_with_extra(self): offset += len(json_extra) self.verify_time_tgt_pkt_buffer(offset, time, pkt) + def test_write_encodes_binary_extra(self): + # Binary values reach a command's extra via cmd(extra=...), so the protocol has + # to use JsonEncoder like the rest of COSMOS. Plain json.dumps raises + # "Object of type bytes is not JSON serializable" and kills the write. + _time, pkt = self.setup_stream_pkt() + pkt.stored = False + pkt.extra = {"data": b"\xff"} + self.interface.write(pkt) + + json_extra = json.dumps({"data": b"\xff"}, cls=JsonEncoder).encode("utf-8") + offset = 1 + self.assertEqual( + struct.unpack(">I", TestPreidentifiedProtocol.buffer[offset : (offset + 4)])[0], + len(json_extra), + ) + offset += 4 + written = TestPreidentifiedProtocol.buffer[offset : (offset + len(json_extra))] + self.assertEqual(json.loads(written, cls=JsonDecoder), {"data": b"\xff"}) + def test_write_creates_a_packet_header_with_stored_and_extra(self): time, pkt = self.setup_stream_pkt() pkt.stored = True diff --git a/openc3/spec/interfaces/protocols/preidentified_protocol_spec.rb b/openc3/spec/interfaces/protocols/preidentified_protocol_spec.rb index b6c043cf3c..0888bee33f 100644 --- a/openc3/spec/interfaces/protocols/preidentified_protocol_spec.rb +++ b/openc3/spec/interfaces/protocols/preidentified_protocol_spec.rb @@ -160,6 +160,26 @@ def write(data); $buffer = data; end expect($buffer[offset..-1]).to eql pkt.buffer end + it "writes a byte accurate length for non-ASCII extra" do + @interface.instance_variable_set(:@stream, PreStream.new) + @interface.add_protocol(PreidentifiedProtocol, [nil, 5], :READ_WRITE) + pkt = System.telemetry.packet("SYSTEM", "META").clone + pkt.received_time = Time.new(2020, 1, 31, 12, 15, 30.5) + pkt.stored = false + # Multi-byte UTF-8 makes String#length differ from String#bytesize. The + # length field counts bytes, so a character count would desync the receiver. + extra_data = { "note" => "café" } + pkt.extra = extra_data + @interface.write(pkt) + + json_extra = extra_data.as_json().to_json(allow_nan: true) + expect(json_extra.bytesize).to_not eql json_extra.length + offset = 1 # flags + expect($buffer[offset..(offset + 3)].unpack('N')[0]).to eql json_extra.bytesize + offset += 4 + expect($buffer[offset...(offset + json_extra.bytesize)]).to eql json_extra.b + end + it "creates a packet header with stored and extra" do @interface.instance_variable_set(:@stream, PreStream.new) @interface.add_protocol(PreidentifiedProtocol, [nil, 5], :READ_WRITE) diff --git a/openc3/spec/logs/packet_log_writer_spec.rb b/openc3/spec/logs/packet_log_writer_spec.rb index b79ece2345..8ae451bd71 100644 --- a/openc3/spec/logs/packet_log_writer_spec.rb +++ b/openc3/spec/logs/packet_log_writer_spec.rb @@ -219,6 +219,41 @@ module OpenC3 FileUtils.rm_f 'test_log.bin' end + it "round trips non-ASCII extra" do + # JSON.generate returns UTF-8. Appending that to the binary log entry raises + # Encoding::CompatibilityError once the entry holds a byte >= 0x80 (the packet + # time nearly always does), which silently dropped the packet from the log. + # The extra length field also has to count bytes rather than characters. + now = Time.now.to_nsec_from_epoch + extra = { 'note' => 'café', 'flow_uuid' => '1234-5678' } + encoded = JSON.generate(extra.as_json, allow_nan: true) + expect(encoded.bytesize).to_not eq encoded.length + + plw = PacketLogWriter.new(@log_dir, 'test') + # JSON format so extra goes through JSON.generate. to_cbor already returns a + # binary string where length == bytesize. + plw.data_format = :JSON + plw.write(:RAW_PACKET, :CMD, 'TGT1', 'PKT1', now, false, "\x01\x02", nil, '0-0', extra: extra) + plw.write(:RAW_PACKET, :CMD, 'TGT2', 'PKT2', now, false, "\x03\x04", nil, '0-0') + threads = plw.shutdown + threads.each { |t| t.join } + + expect(@files.keys.length).to eq 1 + bin = Zlib::GzipReader.new(StringIO.new(@files.values.first)).read + File.open('test_log.bin', 'wb') { |file| file.write bin } + reader = PacketLogReader.new + reader.open('test_log.bin') + got = [] + while (pkt = reader.read) + got << [pkt.target_name, pkt.buffer] + end + # The packet carrying extra must be present, and the one after it must still + # be readable, which only happens if the entry length counted bytes + expect(got).to eql [['TGT1', "\x01\x02"], ['TGT2', "\x03\x04"]] + reader.close() + FileUtils.rm_f 'test_log.bin' + end + it "correctly writes multiple files in a row" do first_time = Time.now.to_nsec_from_epoch last_time = first_time += 1_000_000_000 From 795e757d6539660220beea23de5315e3dacc5055 Mon Sep 17 00:00:00 2001 From: Jason Thomas Date: Wed, 9 Sep 2026 17:55:10 -0600 Subject: [PATCH 7/7] fix(python): decode Ruby encoded special floats in JsonDecoder Ruby's Float#as_json writes non-finite floats as {"json_class": "Float", "raw": "NaN"|"Infinity"|"-Infinity"} since bare NaN/Infinity literals are not valid JSON. JsonDecoder only understood the String form, so those values reached Python interfaces as dicts rather than floats. Commands released by the Ruby queue microservice carry their extra in exactly this encoding. Decode both forms. The encoder is unchanged: Python writes bare literals that Ruby already parses with allow_nan, and switching it would break readers using plain json.loads. Co-Authored-By: Claude Opus 5 (1M context) --- openc3/python/openc3/utilities/json.py | 18 ++++++++++++- openc3/python/test/utilities/test_json.py | 33 +++++++++++++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/openc3/python/openc3/utilities/json.py b/openc3/python/openc3/utilities/json.py index c4f844056e..ec3ded428a 100644 --- a/openc3/python/openc3/utilities/json.py +++ b/openc3/python/openc3/utilities/json.py @@ -26,10 +26,26 @@ def default(self, o): class JsonDecoder(json.JSONDecoder): + # Ruby's Float#as_json (openc3/io/json_rpc.rb) encodes non-finite floats as + # {"json_class": "Float", "raw": "NaN"|"Infinity"|"-Infinity"} because bare + # NaN/Infinity literals are not valid JSON. Python's json module writes those + # bare literals instead and reads them back natively, so only the decode side + # needs to understand both forms. + RUBY_SPECIAL_FLOATS = { + "NaN": float("nan"), + "Infinity": float("inf"), + "-Infinity": float("-inf"), + } + def __init__(self, *args, **kwargs): json.JSONDecoder.__init__(self, object_hook=self.object_hook, *args, **kwargs) # noqa: B026 def object_hook(self, dct): - if dct.get("json_class") == "String": + json_class = dct.get("json_class") + if json_class == "String": return bytes(dct["raw"]) + if json_class == "Float": + raw = dct.get("raw") + if isinstance(raw, str) and raw in self.RUBY_SPECIAL_FLOATS: + return self.RUBY_SPECIAL_FLOATS[raw] return dct diff --git a/openc3/python/test/utilities/test_json.py b/openc3/python/test/utilities/test_json.py index 313e76b0d0..8edc793110 100644 --- a/openc3/python/test/utilities/test_json.py +++ b/openc3/python/test/utilities/test_json.py @@ -10,6 +10,7 @@ # if purchased from OpenC3, Inc. import json +import math import unittest from datetime import datetime from unittest.mock import * @@ -31,3 +32,35 @@ def test_encodes_bytearray(self): self.assertEqual(string, '{"json_class": "String", "raw": [0, 1, 2, 3]}') new_ba = json.loads(string, cls=JsonDecoder) self.assertEqual(new_ba, ba) + + def test_decodes_ruby_special_floats(self): + # Ruby's Float#as_json (openc3/io/json_rpc.rb) encodes non-finite floats as a + # json_class Hash since bare NaN/Infinity are not valid JSON. Anything written + # by Ruby and read by Python — a command's extra released from the Ruby queue + # microservice, for one — arrives in this form. + ruby_json = ( + '{"nan": {"json_class": "Float", "raw": "NaN"}, ' + '"inf": {"json_class": "Float", "raw": "Infinity"}, ' + '"ninf": {"json_class": "Float", "raw": "-Infinity"}, ' + '"normal": 1.5}' + ) + decoded = json.loads(ruby_json, cls=JsonDecoder) + self.assertTrue(math.isnan(decoded["nan"])) + self.assertEqual(decoded["inf"], float("inf")) + self.assertEqual(decoded["ninf"], float("-inf")) + self.assertEqual(decoded["normal"], 1.5) + + def test_decodes_python_special_floats(self): + # Python's json module writes bare NaN/Infinity literals and reads them back + # natively, so both forms have to decode to the same values + string = json.dumps({"nan": float("nan"), "inf": float("inf"), "ninf": float("-inf")}, cls=JsonEncoder) + decoded = json.loads(string, cls=JsonDecoder) + self.assertTrue(math.isnan(decoded["nan"])) + self.assertEqual(decoded["inf"], float("inf")) + self.assertEqual(decoded["ninf"], float("-inf")) + + def test_leaves_unknown_json_class_hashes_alone(self): + decoded = json.loads('{"json_class": "Float", "raw": "bogus"}', cls=JsonDecoder) + self.assertEqual(decoded, {"json_class": "Float", "raw": "bogus"}) + decoded = json.loads('{"json_class": "Object", "raw": [1, 2]}', cls=JsonDecoder) + self.assertEqual(decoded, {"json_class": "Object", "raw": [1, 2]})