Merge branch 'horse-cockroach-43481eda' into security-hardening-agent-workflows

Author: mconflitti-pbc

rsconnect/device_login.py

@@ -770,6 +770,10 @@ def _start_result(state: dict[str, Any]) -> dict[str, Any]:
     }
 
 
+def _device_code_expired(state: dict[str, Any]) -> bool:
+    return state["tokens"] is None and state["expires_at"] <= time.time()
+
+
 def start_connect_login(
     url: str,
     name: str,
@@ -799,7 +803,7 @@ def _start_connect_login(
     store = _store()
     _assert_nickname_target(store, "connect", name, server)
     state = _read_state("connect", name)
-    if state and state["expires_at"] <= time.time():
+    if state and _device_code_expired(state):
         _remove_state("connect", name)
         state = None
     if state:
@@ -833,6 +837,7 @@ def _start_connect_login(
             "OAuth client ID",
         )
     client_id, response = _start_device_request(metadata, server, client_id, None, insecure, ca_data, deadline)
+    _remaining(deadline)
     state = _make_state(
         "connect", name, server, metadata, response, client_id, None, set_default, insecure, ca_data, None
     )
@@ -864,7 +869,7 @@ def _start_cloud_login(
     store = _store()
     _assert_nickname_target(store, "cloud", name, server, account)
     state = _read_state("cloud", name)
-    if state and state["expires_at"] <= time.time():
+    if state and _device_code_expired(state):
         _remove_state("cloud", name)
         state = None
     if state:
@@ -878,6 +883,7 @@ def _start_cloud_login(
     client_id = _identifier(connect_cloud.client_id(environment), "OAuth client ID")
     deadline = time.monotonic() + _START_TIMEOUT
     client_id, response = _start_device_request(metadata, server, client_id, connect_cloud.SCOPE, False, None, deadline)
+    _remaining(deadline)
     state = _make_state(
         "cloud",
         name,
@@ -1167,7 +1173,7 @@ def _load_pending(kind: str, name: str) -> dict[str, Any]:
     state = _read_state(kind, name)
     if state is None:
         raise RSConnectException('No pending "%s" login exists for nickname "%s".' % (kind, name))
-    if state["tokens"] is None and state["expires_at"] <= time.time():
+    if _device_code_expired(state):
         _remove_state(kind, name)
         raise _Expired()
     _assert_nickname_target(_store(), kind, name, state["server"], state["account"])

rsconnect/http_support.py

@@ -638,7 +638,7 @@ def interrupt_request() -> None:
                     raise socket.timeout("HTTP request deadline exceeded.")
             response_body = response.read()
             with request_lock:
-                self._apply_request_deadline()
+                # A complete token response must reach its checkpoint even after the deadline.
                 request_completed = True
             return response, response_body
         except Exception:

rsconnect/preflight.py

@@ -155,16 +155,24 @@ def _deployment_store_paths(target: str) -> list[str]:
 def load_preflight_app_store(path: str) -> AppStore:
     """Load safe deployment history for executor target inference."""
     module_file = fake_module_file_from_directory(path)
-    try:
-        store = AppStore(module_file, strict_read=True)
-        records = store.get_all()
-        if not isinstance(records, list) or any(
-            not isinstance(record, Mapping) or not isinstance(record.get("server_url"), str) or not record["server_url"]
-            for record in records
-        ):
-            raise TypeError("Malformed local deployment metadata.")
-    except _APP_STORE_READ_ERRORS:
-        return AppStore(module_file, autoload=False, strict_read=True)
+    store_paths = [path, module_file] if os.path.isfile(path) else [module_file]
+    store = AppStore(module_file, autoload=False, strict_read=True)
+    for store_path in store_paths:
+        try:
+            candidate = AppStore(store_path, strict_read=True)
+            records = candidate.get_all()
+            if not isinstance(records, list) or any(
+                not isinstance(record, Mapping)
+                or not isinstance(record.get("server_url"), str)
+                or not record["server_url"]
+                for record in records
+            ):
+                raise TypeError("Malformed local deployment metadata.")
+        except _APP_STORE_READ_ERRORS:
+            return AppStore(store_path, autoload=False, strict_read=True)
+        store = candidate
+        if records:
+            return store
     return store
 
 

rsconnect/preflight_node.py

@@ -176,11 +176,16 @@ def _evaluate_node_range(
     return _parse_node_evaluation(result.stdout, len(versions))
 
 
-def _server_status(value: Any, warnings: List[str]) -> Tuple[Optional[Dict[str, Any]], bool]:
+def _server_status(value: Any, warnings: List[str], actions: List[str]) -> Tuple[Optional[Dict[str, Any]], bool]:
     if not isinstance(value, MappingABC):
-        warnings.append("Connect did not return Node.js status metadata; compatibility is unknown.")
+        warnings.append("Connect did not return Node.js runtime status flags; compatibility is unknown.")
+        actions.append("Check whether this Connect server exposes Node.js runtime status flags.")
         return None, False
     status = dict(cast(Mapping[str, Any], value))
+    if not any(field in status for field in _NODEJS_STATUS_FLAGS):
+        warnings.append("Connect did not return Node.js runtime status flags; compatibility is unknown.")
+        actions.append("Check whether this Connect server exposes Node.js runtime status flags.")
+        return status, False
     complete = True
     for field in _NODEJS_STATUS_FLAGS:
         if field not in status:
@@ -222,30 +227,34 @@ def _server_installations(value: Any, warnings: List[str]) -> Tuple[List[Tuple[s
     return installations, metadata_complete, publishability_complete
 
 
-def _server_nodejs_info(settings: Any, warnings: List[str]) -> Dict[str, Any]:
+def _server_nodejs_info(settings: Any, warnings: List[str], actions: List[str]) -> Dict[str, Any]:
     info: Dict[str, Any] = {
         "enabled": None,
         "status": None,
         "installations": [],
-        "metadata_complete": False,
+        "status_complete": False,
+        "runtime_complete": False,
         "publishability_complete": False,
     }
     if not isinstance(settings, MappingABC):
         warnings.append("Connect did not return Node.js settings; compatibility is unknown.")
+        actions.append("Check whether this Connect server exposes Node.js runtime settings.")
         return info
 
     settings = cast(Mapping[str, Any], settings)
     enabled = settings.get("enabled")
     if not isinstance(enabled, bool):
         warnings.append("Connect did not report whether Node.js is enabled; compatibility is unknown.")
-    status, status_complete = _server_status(settings.get("status"), warnings)
+        actions.append("Check whether Node.js is enabled in the Connect server settings.")
+    status, status_complete = _server_status(settings.get("status"), warnings, actions)
     installations, installations_complete, publishability_complete = _server_installations(
         settings.get("installations"), warnings
     )
     info["enabled"] = enabled if isinstance(enabled, bool) else None
     info["status"] = status
     info["installations"] = installations
-    info["metadata_complete"] = isinstance(enabled, bool) and status_complete and installations_complete
+    info["status_complete"] = status_complete
+    info["runtime_complete"] = isinstance(enabled, bool) and installations_complete
     info["publishability_complete"] = publishability_complete
     return info
 
@@ -283,7 +292,7 @@ def _unmatched_server_compatibility(
 def _server_compatibility(info: Dict[str, Any], is_new: bool, evaluation: Optional[Dict[str, Any]]) -> str:
     if info["enabled"] is False or _nodejs_status_failures(info):
         return "incompatible"
-    if info["enabled"] is not True or not info["metadata_complete"]:
+    if not info["runtime_complete"]:
         return "unknown"
 
     installations = info["installations"]
@@ -294,7 +303,7 @@ def _server_compatibility(info: Dict[str, Any], is_new: bool, evaluation: Option
     if not _valid_node_evaluation(evaluation):
         return "unknown"
     if _has_matching_installation(indices, cast(Dict[str, Any], evaluation)):
-        return "compatible"
+        return "compatible" if info["status_complete"] else "unknown"
     return _unmatched_server_compatibility(indices, is_new, info, cast(Dict[str, Any], evaluation))
 
 
@@ -359,9 +368,8 @@ def _read_server_nodejs_info(client: RSConnectClient, warnings: List[str], actio
         settings = client.nodejs_settings()
     except RSConnectException as err:
         warnings.append(f"Could not read server Node.js settings: {err}")
-        actions.append("Check whether this Connect server exposes Node.js runtime settings.")
         settings = None
-    return _server_nodejs_info(settings, warnings)
+    return _server_nodejs_info(settings, warnings, actions)
 
 
 def _evaluate_project_range(

tests/test_agent_cli_e2e.py

@@ -41,6 +41,21 @@
         logs=cloud_base + "/v1",
     )
 
+token_read_delay = os.environ.get("RSCONNECT_E2E_TOKEN_READ_DELAY")
+if token_read_delay:
+    import http.client
+    import time
+
+    original_read = http.client.HTTPResponse.read
+
+    def read_completed_token(self, *args, **kwargs):
+        body = original_read(self, *args, **kwargs)
+        if self.status == 200 and b'"access_token"' in body:
+            time.sleep(float(token_read_delay))
+        return body
+
+    http.client.HTTPResponse.read = read_completed_token
+
 runpy.run_module("rsconnect.main", run_name="__main__")
 """
 
@@ -360,6 +375,7 @@ def _cli_environment(home: Path) -> dict[str, str]:
         "CONNECT_CLOUD_OAUTH_CLIENT_ID",
         "SHINYAPPS_ACCOUNT",
         "RSCONNECT_E2E_CLOUD_BASE_URL",
+        "RSCONNECT_E2E_TOKEN_READ_DELAY",
         "HTTP_PROXY",
         "http_proxy",
         "HTTPS_PROXY",
@@ -1327,6 +1343,59 @@ def test_cloud_finish_deadline_covers_delayed_refresh_and_keeps_checkpoint(
     assert saved["connect_cloud_refresh_token"] == "refresh-token"
 
 
+@pytest.mark.parametrize("kind", ["connect", "cloud"])
+def test_completed_token_response_is_checkpointed_after_deadline_and_device_expiry(
+    tmp_path: Path, local_http_server: _LocalHTTPServer, kind: str
+) -> None:
+    home = tmp_path / "home"
+    environment = _cli_environment(home)
+    name = "completed-token-" + kind
+    if kind == "cloud":
+        start_args = ["add", "--connect-cloud", "--account", "team", "--name", name, "--no-wait"]
+        finish_args = ["add", "--connect-cloud", "--name", name, "--finish"]
+        cloud_base = local_http_server.base_url
+    else:
+        start_args = ["login", "--server", local_http_server.base_url, "--name", name, "--no-wait"]
+        finish_args = ["login", "--name", name, "--finish"]
+        cloud_base = None
+
+    started = _run_cli(start_args, environment, cloud_base_url=cloud_base)
+    assert started.returncode == 0, _output(started)
+    local_http_server.approved = True
+    time.sleep(1.1)
+    delayed_environment = {**environment, "RSCONNECT_E2E_TOKEN_READ_DELAY": "1.2"}
+    pending = _run_cli([*finish_args, "--timeout", "1"], delayed_environment, cloud_base_url=cloud_base)
+
+    assert pending.returncode == 0, _output(pending)
+    assert _json_output(pending)["status"] == "pending"
+    state_path = _device_states(home, kind)[0]
+    state = json.loads(state_path.read_text(encoding="utf-8"))
+    assert state["tokens"]["access_token"] == "access-token"
+    assert state["tokens"]["refresh_token"] == "refresh-token"
+    state["expires_at"] = time.time() - 1
+    state_path.write_text(json.dumps(state), encoding="utf-8")
+    local_http_server.device_error = "invalid_grant"
+
+    restarted = _run_cli(start_args, environment, cloud_base_url=cloud_base)
+    assert restarted.returncode == 0, _output(restarted)
+    assert _json_output(restarted)["expires_in"] == 0
+    assert json.loads(state_path.read_text(encoding="utf-8"))["tokens"] == state["tokens"]
+    assert len(_requests(local_http_server, "/oauth/device/authorize")) == 1
+
+    finished = _run_cli([*finish_args, "--timeout", "5"], environment, cloud_base_url=cloud_base)
+    assert finished.returncode == 0, _output(finished)
+    assert _json_output(finished)["status"] == "done"
+    assert not _device_states(home, kind)
+    assert len(_requests(local_http_server, "/oauth/token")) == 1
+    saved = _saved_servers(home)[name]
+    token_field = "connect_cloud_access_token" if kind == "cloud" else "oauth_access_token"
+    assert saved[token_field] == "access-token"
+    for result in (started, pending, restarted, finished):
+        assert "access-token" not in _output(result)
+        assert "refresh-token" not in _output(result)
+        assert "local-device-code" not in _output(result)
+
+
 @pytest.mark.parametrize("phase", ["headers", "body"])
 @pytest.mark.parametrize("kind", ["connect", "cloud"])
 def test_finish_deadline_interrupts_slow_responses_and_can_resume(
@@ -1780,7 +1849,7 @@ def test_oversized_valid_deployment_record_is_unknown_and_explicit_id_bypasses_i
 @pytest.mark.parametrize(
     "contents, warning_text",
     [
-        pytest.param("[" * 10000 + "0" + "]" * 10000, "recursion", id="nested-json"),
+        pytest.param("[" * 10000 + "0" + "]" * 10000, "deployment metadata", id="nested-json"),
         pytest.param(
             '{"app_id":' + "9" * 5000 + "}",
             "integer string conversion",
@@ -1792,7 +1861,7 @@ def test_oversized_valid_deployment_record_is_unknown_and_explicit_id_bypasses_i
         ),
     ],
 )
-def test_parser_rejected_deployment_record_is_unknown_without_fixing(
+def test_unusable_deployment_record_is_unknown_without_fixing(
     tmp_path: Path, local_http_server: _LocalHTTPServer, runtime: str, contents: str, warning_text: str
 ) -> None:
     environment = _cli_environment(tmp_path / "home")
@@ -1818,6 +1887,7 @@ def test_parser_rejected_deployment_record_is_unknown_without_fixing(
     assert report["changed_files"] == []
     assert report["actions"]
     assert any(warning_text in warning.lower() for warning in report["warnings"])
+    assert record.read_text(encoding="utf-8") == contents
     assert not (project / ".python-version").exists()
     assert not _requests(local_http_server, "/__api__/v1/content/" + _CONTENT_GUID)
 
@@ -1876,8 +1946,9 @@ def test_resumable_reauthentication_preserves_legacy_url_and_content_history(
     assert not (project / ".python-version").exists()
 
 
+@pytest.mark.parametrize("filename", [None, "app.py", "manifest.json"])
 def test_safe_deployment_history_still_selects_its_server_before_the_default(
-    tmp_path: Path, local_http_server: _LocalHTTPServer
+    tmp_path: Path, local_http_server: _LocalHTTPServer, filename: str | None
 ) -> None:
     home = tmp_path / "home"
     environment = _cli_environment(home)
@@ -1893,7 +1964,9 @@ def test_safe_deployment_history_still_selects_its_server_before_the_default(
     next(home.rglob("servers.json")).write_text(json.dumps(servers), encoding="utf-8")
     project = tmp_path / "history-target-project"
     project.mkdir()
-    module_file = project / (project.name + ".py")
+    module_file = project / (filename or project.name + ".py")
+    if filename:
+        module_file.write_text("{}" if filename.endswith(".json") else "app = object()\n", encoding="utf-8")
     AppStore(str(module_file)).set(
         local_http_server.base_url,
         str(module_file),
@@ -1904,7 +1977,8 @@ def test_safe_deployment_history_still_selects_its_server_before_the_default(
         "python-api",
     )
 
-    result = _run_cli(["preflight", str(project), "--fix"], environment)
+    target = module_file if filename else project
+    result = _run_cli(["preflight", str(target), "--fix"], environment)
 
     assert result.returncode == 0, _output(result)
     report = _json_output(result)