chore(agent-tunnel): address review feedback on PR

* `upstream::ConnectedUpstream::server_addr` for tunneled targets now
  reports the target IP:port (or `0.0.0.0:<port>` for a hostname
  target) instead of `0.0.0.0:0`. Logs / PCAP filenames / session info
  surface a meaningful peer address again.
* `RoutePlan` and its methods are now `pub(crate)`; they were never
  meant to leak outside the upstream module.
* `RoutingDecision::ExplicitAgentNotFound` is logged and degraded to
  Direct rather than panicking via `unreachable!` — the routing crate
  could grow new branches and we should not crash the gateway on a
  contract drift.
* `/jet/tunnel/enrollment-string` now returns `400` when neither
  `quic_host` is provided nor a parseable host can be extracted from
  `api_base_url`. The previous silent fallback to `conf.hostname` was
  a Docker/K8s footgun (often a container ID the agent cannot dial),
  and the token store insert is now performed only after that check
  so a 400 leaves no orphan token.
* `AgentManagementReadAccess` and `AgentManagementWriteAccess` reject
  with an error message that names the accepted scopes, easing
  integration debugging.

Author: irvingoujAtDevolution

devolutions-gateway/src/api/tunnel.rs

@@ -304,25 +304,33 @@ async fn create_agent_enrollment_string(
 
     let lifetime_secs = req.lifetime.unwrap_or(3600);
 
-    // Generate a one-time enrollment token stored server-side.
-    let enrollment_token = Uuid::new_v4().to_string();
-    handle
-        .enrollment_token_store()
-        .insert(enrollment_token.clone(), req.name.clone(), Some(lifetime_secs))
-        .await;
-
-    // Determine QUIC host: explicit override > extract from api_base_url > gateway hostname config.
-    // The gateway hostname config is often a container ID in Docker, so we prefer
-    // extracting the host from the api_base_url which the caller already knows is reachable.
+    // Determine QUIC host: explicit override > host extracted from api_base_url.
+    // We deliberately do NOT fall back to `conf.hostname`: in Docker/K8s that is
+    // typically a container ID or pod name not resolvable by the agent, so a
+    // silent fallback would emit an enrollment string the agent cannot use.
+    // Force the caller to either supply `quic_host` or pass an `api_base_url`
+    // we can parse a host out of.
     let quic_host = match req.quic_host.as_deref().filter(|h| !h.is_empty()) {
         Some(host) => host.to_owned(),
         None => url::Url::parse(&req.api_base_url)
             .ok()
             .and_then(|u| u.host_str().map(ToOwned::to_owned))
-            .unwrap_or_else(|| conf.hostname.clone()),
+            .ok_or_else(|| {
+                HttpError::bad_request().msg(
+                    "could not derive QUIC host: api_base_url has no host component, pass `quic_host` explicitly",
+                )
+            })?,
     };
     let quic_endpoint = format!("{quic_host}:{}", conf.agent_tunnel.listen_port);
 
+    // Generate a one-time enrollment token stored server-side. Done after the
+    // quic_host validation so a 400 response does not pollute the store.
+    let enrollment_token = Uuid::new_v4().to_string();
+    handle
+        .enrollment_token_store()
+        .insert(enrollment_token.clone(), req.name.clone(), Some(lifetime_secs))
+        .await;
+
     // Build the enrollment payload.
     let payload = serde_json::json!({
         "version": 1,

devolutions-gateway/src/extract.rs

@@ -434,7 +434,9 @@ where
                 | AccessScope::AgentRead
                 | AccessScope::DiagnosticsRead
                 | AccessScope::ConfigWrite => Ok(Self),
-                _ => Err(HttpError::forbidden().msg("invalid scope for agent management read")),
+                _ => Err(HttpError::forbidden().msg(
+                    "invalid scope for agent management read (require one of: gateway.agent.read, gateway.diagnostics.read, gateway.config.write, *)",
+                )),
             },
             _ => Err(HttpError::forbidden().msg("scope token required for agent management read")),
         }
@@ -466,7 +468,9 @@ where
         match claims {
             AccessTokenClaims::Scope(scope) => match scope.scope {
                 AccessScope::Wildcard | AccessScope::AgentEnroll | AccessScope::ConfigWrite => Ok(Self),
-                _ => Err(HttpError::forbidden().msg("invalid scope for agent management write")),
+                _ => Err(HttpError::forbidden().msg(
+                    "invalid scope for agent management write (require one of: gateway.agent.enroll, gateway.config.write, *)",
+                )),
             },
             _ => Err(HttpError::forbidden().msg("scope token required for agent management write")),
         }

devolutions-gateway/src/upstream.rs

@@ -135,8 +135,11 @@ pub enum UpstreamMode {
 /// or an agent-tunnel stream; a TLS wrap has not yet been applied.
 pub struct ConnectedUpstream {
     pub leg: UpstreamLeg,
-    /// Remote peer address. For agent-tunnel routing this is a placeholder
-    /// (`0.0.0.0:0`) because the real TCP peer lives on the agent side.
+    /// Remote peer address. For direct routing this is the resolved TCP peer.
+    /// For agent-tunnel routing the real TCP peer lives on the agent side, so
+    /// we surface the target IP:port (when the target is an IP literal) or
+    /// `0.0.0.0:<target_port>` (when the target is a hostname the gateway
+    /// never resolves) — both are more useful in logs/PCAP than a true zero.
     pub server_addr: SocketAddr,
     pub selected_target: TargetAddr,
 }
@@ -154,7 +157,7 @@ pub struct PreparedUpstream {
 // ---------------------------------------------------------------------------
 
 /// A routing decision for a single target.
-pub enum RoutePlan<'a> {
+pub(crate) enum RoutePlan<'a> {
     Direct(&'a TargetAddr),
     ViaAgent {
         target: &'a TargetAddr,
@@ -166,7 +169,7 @@ impl<'a> RoutePlan<'a> {
     /// Pick how to reach `target`:
     /// - Explicit `jet_agent_id` → route via that agent (or error if missing).
     /// - Otherwise registry subnet/domain match → best candidates, else Direct.
-    pub async fn resolve(
+    pub(crate) async fn resolve(
         handle: Option<&AgentTunnelHandle>,
         explicit_agent_id: Option<Uuid>,
         target: &'a TargetAddr,
@@ -197,8 +200,15 @@ impl<'a> RoutePlan<'a> {
         match resolve_route(handle.registry(), None, target.host()).await {
             RoutingDecision::ViaAgent(candidates) => Ok(Self::ViaAgent { target, candidates }),
             RoutingDecision::Direct => Ok(Self::Direct(target)),
-            RoutingDecision::ExplicitAgentNotFound(_) => {
-                unreachable!("explicit agent IDs are handled before route resolution")
+            RoutingDecision::ExplicitAgentNotFound(agent_id) => {
+                // resolve_route only returns this when an explicit agent_id is passed
+                // in; we pass None above. Treat as a soft failure rather than panic
+                // so a future change in the routing crate cannot crash the gateway.
+                warn!(
+                    %agent_id,
+                    "routing crate returned ExplicitAgentNotFound for an implicit lookup; falling back to direct"
+                );
+                Ok(Self::Direct(target))
             }
         }
     }
@@ -207,7 +217,7 @@ impl<'a> RoutePlan<'a> {
     ///
     /// For `Direct`, does a TCP connect. For `ViaAgent`, tries each candidate
     /// in order until one succeeds.
-    pub async fn execute(self, handle: Option<&AgentTunnelHandle>, session_id: Uuid) -> Result<ConnectedUpstream> {
+    pub(crate) async fn execute(self, handle: Option<&AgentTunnelHandle>, session_id: Uuid) -> Result<ConnectedUpstream> {
         match self {
             Self::Direct(target) => {
                 trace!(%target, "Connecting to target directly");
@@ -239,8 +249,14 @@ impl<'a> RoutePlan<'a> {
                         .await
                     {
                         Ok(stream) => {
-                            // Real TCP peer lives on the agent; expose a placeholder.
-                            let server_addr: SocketAddr = "0.0.0.0:0".parse().expect("valid placeholder");
+                            // The TCP peer lives on the agent side; surface the target
+                            // IP:port for logs/PCAP when the target is a literal IP, or
+                            // 0.0.0.0:<port> when it's a hostname the gateway never
+                            // resolved itself. Either is more useful than 0.0.0.0:0.
+                            let server_addr = match target.host_ip() {
+                                Some(ip) => SocketAddr::new(ip, target.port()),
+                                None => SocketAddr::from((std::net::Ipv4Addr::UNSPECIFIED, target.port())),
+                            };
 
                             return Ok(ConnectedUpstream {
                                 leg: UpstreamLeg::Tunnel(stream),