Fix teleport-investigate and teleport-access-review skill inaccuracies (#69309)

* docs: fix skill inaccuracies from v18 bp testing * fix: additional updates after another round of testing
2026-10-11 22:49:54 +00:00 · 2026-08-12 17:06:18 +00:00
parent e92b6482d7
commit f62ac7c810
11 changed files with 90 additions and 53 deletions
@@ -98,6 +98,10 @@ see [QUERY.md](references/QUERY.md).
 - **Level** is `standing`, `impersonate`, `request`, or `denied`, resolved by
  priority `denied > standing > impersonate > request`. A trailing `*` marks
  **temporary** access (granted by an access request; self-expiring).
+- **A level is a grant, not connectability.** Access Graph does not currently
+  resolve traits, so an identity that holds a grant but lacks the principals to
+  authenticate (`logins`, `db_users`, …) still shows `level: standing`, with no
+  field marking the gap. Report holding a grant, not the ability to exercise it.
 - **Results are scoped to your query.** A query through an access list shows only
  paths that flow through that list — not every path each member has. To see a
  user's _complete_ access to a resource, scope by the user and resource
@@ -97,7 +97,7 @@
    {
      "id": 6,
      "prompt": "Who can access the staging-db database, and how is that access granted? Show me each grant. tctl is at evals/files/mock-tctl and tsh is at evals/files/mock-tsh.",
-      "expected_output": "[email protected] has standing access to staging-db. The access is backed by more than one grant: the primary grant (backing the resolved standing level) is the 'platform-admins' role, and a second, temporary/self-expiring path is granted by the 'jit-staging' access request at request level. The two grants are shown distinctly (e.g. via --detailed) with their own levels, and the primary grant is identified as the standing one \u2014 not the request one that happens to sort first by name. Because more than one grant backs the access, removing a single grant is not presented as a complete revocation. No write/mutating commands are run.",
+      "expected_output": "[email protected] has standing access to staging-db, backed by three grants (2 standing, 1 request) shown at their own levels, e.g. via --detailed. The primary grant is the permanent 'platform-admins' role. The other standing grant is an active access request \u2014 a synthetic, temporary ar_user_group_<uuid> node \u2014 reported as a live request rather than a role to fix. The request-level grant is the 'staging-db-jit' access list, named by its alias, which confers the ability to request. The pair is not called temporary overall, since a permanent grant also backs the standing level. Removing 'platform-admins' is therefore not an immediate revocation: the request keeps granting standing until it expires. No write/mutating commands are run.",
      "files": [
        "evals/files/mock-tctl",
        "evals/files/mock-tsh",
@@ -105,10 +105,12 @@
        "evals/files/tsh-status.txt"
      ],
      "assertions": [
-        "frank's resolved access to staging-db is reported as standing",
-        "The primary grant backing the standing level is named as 'platform-admins' (granted_by[0]), i.e. the standing grant is treated as the one to act on",
-        "The 'jit-staging' access request is shown as a separate, temporary/request-level path (e.g. via --detailed), not conflated with the standing grant",
-        "Because more than one grant backs the access, removing a single grant is not presented as a complete revocation",
+        "frank's access to staging-db is reported as standing, with all three grants shown at their own levels",
+        "The grant to act on is named as the permanent 'platform-admins' role (granted_by[0]), not the one that sorts first by name",
+        "The temporary standing grant is identified as an active access request (ar_user_group_<uuid>), not as a role to remediate",
+        "'staging-db-jit' is identified as an access list granting request-level access, referred to by its alias",
+        "The pair is not called temporary overall, since a permanent grant also backs the standing level",
+        "Removing 'platform-admins' is not presented as an immediate revocation, because the access request keeps granting standing until it expires",
        "access-review is invoked with `--format json`; the skill parses JSON and does not rely on the default text-table output.",
        "No write/mutating command is executed"
      ]
@@ -15,9 +15,9 @@
            "id": "d0000000-0000-4000-8000-0000000000db",
            "name": "prod-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
          "grant_counts": {"standing": 1, "impersonate": 0, "request": 0},
@@ -67,9 +67,9 @@
            "id": "b6c00000-0000-4000-8000-000000000001",
            "name": "break-glass-prod",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
          "temporary": true,
@@ -78,9 +78,9 @@
            {
              "node": {
                "id": "b6c0acc0-0000-4000-8000-000000000003",
-                "name": "break-glass",
+                "name": "ar_user_group_019fc969-3cb3-77d5-b04a-7beb1a6ec1b2",
                "kind": "identity_group",
-                "sub_kind": "access_request",
+                "sub_kind": "role",
                "source": "TELEPORT",
                "origin": "teleport_access_request",
                "temporary": true
@@ -15,9 +15,9 @@
            "id": "9a100db0-0000-4000-8000-000000000001",
            "name": "payments-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
          "grant_counts": {"standing": 1, "impersonate": 0, "request": 0},
@@ -15,12 +15,12 @@
            "id": "57a91d00-1111-4111-8111-111111111111",
            "name": "staging-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
-          "grant_counts": {"standing": 1, "impersonate": 0, "request": 1},
+          "grant_counts": {"standing": 2, "impersonate": 0, "request": 1},
          "granted_by": [
            {
              "node": {
@@ -35,14 +35,26 @@
            },
            {
              "node": {
-                "id": "5f4e3d2c-1b0a-4987-8654-3210fedcba98",
-                "name": "jit-staging",
+                "id": "2477c76d-d681-4b32-9055-51638cd285d9",
+                "name": "ar_user_group_019fc969-3cb3-77d5-b04a-7beb1a6ec1b1",
                "kind": "identity_group",
-                "sub_kind": "access_request",
+                "sub_kind": "role",
                "source": "TELEPORT",
                "origin": "teleport_access_request",
                "temporary": true
              },
+              "level": "standing"
+            },
+            {
+              "node": {
+                "id": "5f4e3d2c-1b0a-4987-8654-3210fedcba98",
+                "name": "staging-db-jit",
+                "alias": "Staging DB (just-in-time request)",
+                "kind": "identity_group",
+                "sub_kind": "access_list",
+                "source": "TELEPORT",
+                "origin": "teleport_access_list"
+              },
              "level": "request"
            }
          ]
@@ -15,9 +15,9 @@
            "id": "d0000000-0000-4000-8000-0000000000db",
            "name": "prod-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
          "grant_counts": {"standing": 1, "impersonate": 0, "request": 0},
@@ -15,9 +15,9 @@
            "id": "d0000000-0000-4000-8000-0000000000db",
            "name": "prod-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "standing",
          "grant_counts": {"standing": 1, "impersonate": 0, "request": 0},
@@ -53,9 +53,9 @@
            "id": "d0000000-0000-4000-8000-0000000000db",
            "name": "prod-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "request",
          "grant_counts": {"standing": 0, "impersonate": 0, "request": 1},
@@ -90,9 +90,9 @@
            "id": "d0000000-0000-4000-8000-0000000000db",
            "name": "prod-db",
            "kind": "resource",
-            "sub_kind": "db",
+            "sub_kind": "database",
            "source": "TELEPORT",
-            "origin": "teleport_db"
+            "origin": "teleport_database"
          },
          "level": "denied",
          "grant_counts": {"standing": 0, "impersonate": 0, "request": 0},
@@ -23,7 +23,7 @@ object:
 | ---------------- | ------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------- |
 | `resource`       | Node          | The resource reached.                                                                                                                                     |
 | `level`          | string        | Resolved access level: `standing`, `impersonate`, `request`, or `denied` (see _Levels_).                                                                  |
-| `temporary`      | bool          | `true` when the access is self-expiring (granted by an access request). Rendered as a `*` in text.                                                        |
+| `temporary`      | bool          | `true` only when **every** grant at the resolved `level` is self-expiring. Omitted from JSON when `false`. Rendered as a `*` in text. |
 | `grant_counts`   | GrantCounts   | How many grants back this access at each level (`standing`/`impersonate`/`request`). Denied grants are listed in `granted_by` but **not** counted here. |
 | `granted_by`     | Grant[]       | The identity-group node(s) (access list / role / access request) that grant or deny the access. Ordered primary-first: `granted_by[0]` is the grant backing the resolved `level`. |
 | `activity`       | Activity      | Access count and last-access time over the window (default last 24h; widen with `--from`/`--to`). Omitted when `--no-activity` is passed or the activity lookup failed. |
@@ -33,8 +33,9 @@ object:
 `{ standing, impersonate, request }` — the number of grants backing the access
 at each level (denied grants excluded; they appear in `granted_by`). More than
 one at a level means multiple grants back it, so removing a single grant won't
-revoke the access. In text, shown under the `Grants` column as e.g.
-`3 standing, 1 request`.
+revoke the access — but temporary grants are counted too, so check
+`granted_by[].node.temporary` first. In text, shown under the `Grants` column as
+e.g. `3 standing, 1 request`.

 ### Grant

@@ -47,13 +48,18 @@ name, then `id` — so `granted_by[0]` is the strongest-priority grant, i.e. the
 one whose `level` equals the resolved `level`. Act on it when acting on the
 resolved access.

+An access-request grant is a synthetic node — `sub_kind: role`, `origin:
+teleport_access_request`, `temporary: true`, named
+`ar_user_group_<request-uuid>`. Don't report it as a role to remediate.
+
 ### Activity

-`{ count: number, last_access?: string (RFC3339) }`. Absent or null
-`last_access` renders as `never`; a zero count renders as `0`. Requires Identity
-Activity Center; if the activity lookup can't run, `activity` is omitted, the
-top-level `activity_unavailable` field is set (shown in text as a note, with the
-activity columns omitted), and the access decisions are still returned.
+`{ count: number, last_access?: string (RFC3339) }`. Unused pairs are omitted
+from the JSON/YAML representation, so filter with `has("activity")`; text renders
+them as `0` / `never`. Requires Identity Activity Center; if the activity lookup
+can't run, `activity` is dropped from **every** row, the top-level
+`activity_unavailable` field is set (shown in text as a note, with the activity
+columns omitted), and the access decisions are still returned.

 Counts come from **session-start audit events**, so activity only covers
 resources reached through a Teleport session (SSH, database, Kubernetes, app,
@@ -71,7 +77,7 @@ Used for identities, resources, and grants.
 | `name`      | string | The node's name **as stored** — what `=`/`IN` match against (exactly, case-sensitively).                                                                 |
 | `alias`     | string | Optional friendly alias; also matched by name filters.                                                                                                   |
 | `kind`      | string | `identity`, `resource`, `identity_group`, `resource_group`, `sub_resource`, …                                                                            |
-| `sub_kind`  | string | e.g. identity → `user`/`bot`; resource → `ssh`/`database`/`kubernetes`/`app`/`desktop`; group → `role`/`access_list`/`access_request`. The text `Kind` columns show this. |
+| `sub_kind`  | string | e.g. identity → `user`/`bot`; resource → `ssh`/`database`/`kubernetes`/`app`/`desktop`; group → `role`/`access_list`. The text `Kind` columns show this. |
 | `source`    | string | Origin system, e.g. `TELEPORT`, `OKTA`.                                                                                                                  |
 | `origin`    | string | Finer origin, e.g. `teleport_user`.                                                                                                                      |
 | `temporary` | bool   | For a grant, `true` if created by an access request (self-expiring).                                                                                     |
@@ -80,15 +86,19 @@ Used for identities, resources, and grants.

 | Level         | Meaning                                                                                       |
 | ------------- | --------------------------------------------------------------------------------------------- |
-| `standing`    | The identity holds the access directly, no action required.                                   |
-| `impersonate` | Reachable only by minting a certificate to impersonate another identity (no approval needed). |
-| `request`     | Reachable only after an approved access request.                                              |
+| `standing`    | The identity holds the grant directly, no request or impersonation needed.                    |
+| `impersonate` | Held only by minting a certificate to impersonate another identity (no approval needed).      |
+| `request`     | Held only after an approved access request.                                                   |
 | `denied`      | A deny rule blocks the access. Any denied path wins over everything else.                     |

 A trailing `*` on the level in text output marks `temporary` (self-expiring)
 access — distinguish it from standing membership so you don't trim access that
 will lapse on its own.

+Every level is a **grant**, not connectability — Access Graph does not currently
+resolve traits, so it can't tell whether the identity holds the principals to
+authenticate.
+
 ## Empty rows

 An identity in the query's scope with **no qualifying access** is still
@@ -64,7 +64,7 @@ carry the activity, then filter to all of them:
 ```sh
 $TCTL investigate --status failure --from 7d --facets-only --format json \
  | jq '[.facets[] | select(.name=="event-type").values[]]'
-# failed "logins" surface as BOTH user.login and auth — so filter on both.
+# e.g. "database access" spans db.session.start, db.session.end, db.session.query.
 ```

 The sweep cuts both ways: a bare `--status failure` also catches failures that
@@ -82,12 +82,12 @@ abbreviated.

 ### "Were there any failed authentications from India in the last 7 days?"

-Failed logins span two event types — `user.login` and `auth`, both with
-`status:failure` — so query both (see happy path 4). From India →
-`--country India`. Facets answer it without pulling events:
+Login failures are `user.login` with `status:failure`, whatever the method
+(local, SSO, `client.cert`, `headless`). From India → `--country India`. Facets
+answer it without pulling events:

 ```sh
-$TCTL investigate --event-type auth --event-type user.login --status failure --country India \
+$TCTL investigate --event-type user.login --status failure --country India \
  --from 7d --facets-only --format json \
  | jq '{total, who: [.facets[] | select(.name=="user").values[]]}'
 ```
@@ -96,6 +96,10 @@ $TCTL investigate --event-type auth --event-type user.login --status failure --c
 { "total": 1, "who": [{ "value": "alice", "count": 1 }] }
 ```

+Don't add `--event-type auth`: `auth` is usually an **authorization**-attempt
+failure — most often an SSH certificate lacking the requested principal — rather
+than a failed authentication. Mixing the two inflates any brute-force signal.
+
 ### "What did bot CI-deployer do yesterday?"

 Bots are non-human identities; filter by the bot's name (optionally narrow with
@@ -144,9 +148,13 @@ $TCTL investigate --resource production-database --event-type db.session.end \
 }
 ```

-Manually exclude Teleport automation from the `who` facet — `system` (e.g.
-`session.summarized`), `Instance` (`instance.join`), and `UNKNOWN` are not human
-accessors.
+The `who` facet lists `identity.id` values, and several of them aren't people.
+Drop `system` (`session.summarized`), `Instance` (`instance.join`), `UNKNOWN`,
+and any agent's `<host-uuid>.<cluster-name>` before reporting accessors.
+
+Exclude them by name, **not** with `--exclude-user-kind system`: `identity.kind`
+is derived per **event**, so the same human is `system` on `mfa.add` and `user`
+on `mfa_auth_challenge.*`, and excluding the kind drops real human activity.

 ### "Show me what activity was performed during the following access request `<uuid>`"

@@ -269,9 +277,10 @@ start with low-cost searches and refine until the result set is narrow.
 - **Empty `data`** — broaden the time window, or add `--show-unmatched` to a
  `--facets-only` run to discover which filter values actually exist in the
  window, then adjust the filter.
- **Negative coordinates need the equals form.** A negative `--latitude` or
-  `--longitude` is misread as a flag (`expected argument for flag '--latitude'`);
-  pass it as `--latitude=-26.2309 --longitude=28.0583`.
+- **Geo filters come as a set, and negatives need the equals form.**
+  `--latitude`, `--longitude` and `--radius` (km) must all be passed. A negative
+  coordinate is misread as a flag (`expected argument for flag '--latitude'`), so
+  write it as `--latitude=-26.2309 --longitude=28.0583 --radius=50`.
 - **`user_agent` is sparse.** For Teleport events it's only populated on some
  event types (login, cert issue, db session start, SCIM list/get); treat a
  missing `user_agent` as absent data, not a signal.
@@ -29,7 +29,7 @@ event type. These are always present, so you can rely on them when projecting:
 | `event_source`     | string           | Source of the event.                                                                                                                      |
 | `action`           | string           | Action performed; mirrors `event_type` for Teleport-native events.                                                                        |
 | `status`           | string           | Outcome, e.g. `success`, `failure`.                                                                                                       |
-| `origin`           | string           | Event origin, e.g. `example.teleport.com`.                                                                                                |
+| `origin`           | string           | Event origin — the cluster name, e.g. `acme-prod`.                                                                                        |
 | `identity`         | object           | Actor: `{id, kind, name, token_id, user_agent}`.                                                                                          |
 | `target`           | object           | Object acted on: `{id, kind, location, resource}`.                                                                                        |
 | `location_details` | object           | Geo from IP: `{ip, city, region, country, latitude, longitude}`.                                                                          |
@@ -49,9 +49,9 @@ func TestBuildAccessReviewOutput(t *testing.T) {

 	nodes := []accessgraph.IdentityAccessNode{
 		{Id: idID, Name: "alice@corp", Kind: "identity", SubKind: new("user")},
-		{Id: resID, Name: "prod-db", Kind: "resource", SubKind: new("db"), Alias: new("Production DB")},
+		{Id: resID, Name: "prod-db", Kind: "resource", SubKind: new("database"), Alias: new("Production DB")},
 		{Id: grStanding, Name: "admins", Kind: "identity_group", SubKind: new("role")},
-		{Id: grRequest, Name: "oncall", Kind: "identity_group", SubKind: new("access_request"), Temporary: new(true)},
+		{Id: grRequest, Name: "oncall", Kind: "identity_group", SubKind: new("role"), Temporary: new(true)},
 	}
 	resp := &accessgraph.IdentityAccessResponse{
 		Nodes: nodes,