feat(access_control): Implement scope validation for per-resource authorization by eveniota · Pull Request #13708 · apache/trafficserver

eveniota · 2026-09-19T09:31:24Z

Summary

The access_control plugin previously parsed and exposed a scope claim and supported the --invalid-scope-status-code configuration option, but AccessToken::validate() never evaluated the scope against incoming request paths. Consequently, any valid token authorized access to every resource covered by a remap rule, and setting --invalid-scope-status-code produced no effect.
This PR finishes the feature by implementing path-prefix scope validation on normalized path segments, providing per-resource granularity while remaining fully backward compatible with existing tokens.

Design Approach (per #13607 suggestion)

This PR implements the minimal backward-compatible approach:

Path-prefix matching on normalized segments
Absent/empty scope = unrestricted (backward compatible)
Scope check in separate function after token validation

These choices are open to feedback; see Design Decisions section below.

Key Requirements & Design Decisions

Clean Separation of Concerns (Plumbing):
- Kept AccessToken::validate() focused purely on token cryptographic integrity, semantics, and timing.
- Implemented scope comparison as a separate helper function, validateScope(requestPath, scope), called in enforceAccessControl() immediately after token->validate() succeeds.
- When out of scope, the transaction state is set to OUT_OF_SCOPE, correctly triggering --invalid-scope-status-code (default: 403) and suppressing subject header extraction.
Matching Semantics (Normalized Path Segments):
- Matching is performed as a path-prefix on normalized segments.
- Handles trailing slashes, redundant slashes, and ATS's TSUrlPathGet() format (which omits leading slashes).
- Enforces directory segment boundaries so that a token scoped to /reports/ (or /reports) authorizes /reports/2026/ but strictly rejects sibling paths like /reports2/ or /reports_backup.
Backward Compatibility (Absent / Empty Scope):
- An absent or empty scope claim is treated as unrestricted. Existing tokens in the field continue to function without disruption.
Resource Granularity vs Target Audience (sub claim):
- Retains the existing model where sub represents target audience (e.g. frogs-in-a-well), while adding the missing per-resource granularity (e.g. scope="/reports/2026/").

What Changed

Core Matching Logic (access_control.cc, access_control.h):
- Declared and implemented validateScope(StringView requestPath, StringView scope).
- Added helper normalizePath() to ensure leading slashes, collapse consecutive slashes, and normalize trailing slashes.
- Updated @todo comments regarding scope validation.
Transaction Enforcement (plugin.cc):
- Inside enforceAccessControl(), extracted the request path via TSUrlPathGet() and validated it against token->getScope().
- On failure, sets data->_vaState = OUT_OF_SCOPE and invokes handleInvalidToken().
Unit Tests (test_access_control.cc):
- Added Catch2 test cases covering:
  - Empty scope (unrestricted / backward compatibility)
  - Exact match (/reports $\rightarrow$ /reports)
  - Subpath matches (/reports $\rightarrow$ /reports/2026/annual.pdf)
  - Sibling segment boundary enforcement (/reports vs /reports2 $\rightarrow$ rejected)
  - Inverted hierarchy / parent path requests with child scope $\rightarrow$ rejected
  - Normalization edge cases (trailing slashes, redundant slashes, root scope "/")
  - Token builder & parser integration with addScope() / getScope()
- Documentation (access_control.en.rst):
  - Replaced the note stating scope is "ignored by the current version of the plugin" with full documentation of the matching semantics, segment boundary rules, and status code behavior.

Future Work

Pattern matching (e.g., /reports/{year}/)
Scope lists or disjunctions
Host-qualified scopes (e.g., example.com:/reports/)
Query parameter scoping

These can be added in follow-ups once the core prefix matching is proven.

Verification

Catch2 unit tests pass: ./build/plugins/experimental/access_control/unit_tests/test_access_control
Code formatted via cmake --build build -t format

Add Catch2 unit tests for validateScope()

Copilot

Copilot review overview

🟡 Changes recommended

Documentation and API wording review comments remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2

Open (2)

Document unrestricted scopes and segment-boundary matching semantics · New
Correct API documentation to say paths fall within scope · New

What changed in this PR

Implements per-resource path-scope validation for the experimental access_control plugin.

Changes:

Adds normalized path-prefix matching with backward-compatible empty scopes.
Enforces scope checks during authorization.
Adds unit tests and updates documentation.

File	Description
`plugins/experimental/access_control/unit_tests/test_access_control.cc`	Scope matching and integration tests
`plugins/experimental/access_control/plugin.cc`	Scope enforcement integration
`plugins/experimental/access_control/access_control.h`	Scope validation API
`plugins/experimental/access_control/access_control.cc`	Path normalization and validation
`doc/admin-guide/plugins/access_control.en.rst`	Scope behavior documentation

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Clarify scope usage in access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Address the traversal vulnerability and add the requested enforcement-path test coverage.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 · 1

Open (2)

Canonicalize dot segments before scope prefix validation · New
Correct API documentation to say paths fall within scope

Resolved since last review (1)

Document unrestricted scopes and segment-boundary matching semantics

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 1

Open (3)

This normalization decodes dot bytes but leaves encoded path separators untouched. For example,… · New
Canonicalize dot segments before scope prefix validation
Correct API documentation to say paths fall within scope

segments

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 3 · 1

Open (4)

String is an alias for std::string, but std::string::starts_with() is only available since… · New
This normalization decodes dot bytes but leaves encoded path separators untouched. For example,…
Canonicalize dot segments before scope prefix validation
Correct API documentation to say paths fall within scope

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical path-decoding and scope-canonicalization issues remain unresolved.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 3 · 1

Open (4)

URL decoder reads past incomplete percent escapes · New
Plus signs incorrectly decoded as spaces in URL paths · New
Scope traversal normalization broadens authorization · New
Correct API documentation to say paths fall within scope

Resolved since last review (3)

String is an alias for std::string, but std::string::starts_with() is only available since…
This normalization decodes dot bytes but leaves encoded path separators untouched. For example,…
Canonicalize dot segments before scope prefix validation

Previously missed (1)

In code that hasn't changed since last review

Add integration coverage for scoped access enforcement

plugins/experimental/access_control/plugin.cc:539

The new enforcement branch is only exercised through direct validateScope() unit calls; the existing access-control AuTest does not send a scoped token through enforceAccessControl() or verify the configured out-of-scope status and subject-header suppression. Add an integration case for an in-scope and out-of-scope request so this transaction-level plumbing is protected.

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 4 · 1 · 3

Open (8)

percentDecodePath() decodes all percent-encoded bytes, including reserved path delimiters like… · New
Scope traversal normalization broadens authorization
Plus signs incorrectly decoded as spaces in URL paths
URL decoder reads past incomplete percent escapes
normalizePath() allocates/constructs multiple strings per request (decoded plus `StringVector… · New
Fix sentence punctuation/capitalization: after optional, this should start a new sentence… · New
The comment reads fails within but should be falls within. · New
Correct API documentation to say paths fall within scope

+static bool
+percentDecodePath(StringView in, String &out)
+{
+  out.clear();
+  out.reserve(in.size());
+  for (size_t i = 0; i < in.size();) {
+    if (in[i] == '%') {
+      unsigned char val = 0;
+      if (i + 2 < in.size() && std::from_chars(in.data() + i + 1, in.data() + i + 3, val, 16).ec == std::errc{}) {
+        out.push_back(static_cast<char>(val));
+        i += 3;
+      } else {
+        return false;
+      }
+    } else {
+      out.push_back(in[i]);
+      ++i;
+    }
+  }
+  return true;
+}


+normalizePath(StringView path, String &normalized, bool isScope = false)
+{
+  normalized.clear();
+  if (path.empty()) {
+    normalized = "/";
+    return true;
+  }
+
+  String decoded;
+  if (!percentDecodePath(path, decoded)) {
+    return false;
+  }
+  StringVector segments;
+  size_t       start = 0;
+
+  while (start < decoded.size()) {
+    size_t end = decoded.find_first_of("/\\", start);
+    if (end == String::npos) {
+      end = decoded.size();
+    }
+    if (end > start) {
+      StringView seg(decoded.data() + start, end - start);
+      if (seg == ".") {
+      } else if (seg == "..") {
+        if (isScope) {
+          return false;
+        }
+        if (!segments.empty()) {
+          segments.pop_back();
+        }
+      } else {
+        segments.emplace_back(seg);
+      }
+    }
+    start = end + 1;
+  }


Clarified the description of the 'scope' parameter in the access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 1 · 5 · 3

Open (9)

percentDecodePath() decodes all percent-encoded bytes, including reserved path delimiters like…
This doc line has a grammatical/capitalization issue (. an absent...) and is overly dense for… · New
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and… · New
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and… · New
This normalization allocates and stores all segments in a vector before rebuilding the normalized… · New
normalizePath() allocates/constructs multiple strings per request (decoded plus `StringVector…
Typo in the public header comment: fails within should be falls within. · New
The comment reads fails within but should be falls within.
Correct API documentation to say paths fall within scope

Resolved since last review (4)

Scope traversal normalization broadens authorization
Plus signs incorrectly decoded as spaces in URL paths
URL decoder reads past incomplete percent escapes
Fix sentence punctuation/capitalization: after optional, this should start a new sentence…

   * ``tid`` for `token id`_, `optional`
   * ``ver`` for `version`_, `optional`, defaults to ``ver=1`` if not specified.
-   * ``scope`` for `scope`_, `optional`, ignored by the current version of the plugin, still not finalized (more applications and their use cases need to be studied to finalize the format)
+   * ``scope`` for `scope`_, `optional`. an absent or empty scope is unrestricted. Otherwise, it is a path-prefix restriction matched against normalized path segments, including the segment boundary (for example, ``/reports`` matches ``/reports/2026`` but not ``/reports2``); out-of-scope requests use the configured ``--invalid-scope-status-code``.


+  for (size_t i = 0; i < in.size();) {
+    if (in[i] == '%') {
+      unsigned char val = 0;
+      if (i + 2 < in.size() && std::from_chars(in.data() + i + 1, in.data() + i + 3, val, 16).ec == std::errc{}) {
+        out.push_back(static_cast<char>(val));
+        i += 3;
+      } else {
+        return false;
+      }
+    } else {
+      out.push_back(in[i]);
+      ++i;
+    }
+  }


+  while (start < decoded.size()) {
+    size_t end = decoded.find_first_of("/\\", start);


+  StringVector segments;
+  size_t       start = 0;
+
+  while (start < decoded.size()) {
+    size_t end = decoded.find_first_of("/\\", start);
+    if (end == String::npos) {
+      end = decoded.size();
+    }
+    if (end > start) {
+      StringView seg(decoded.data() + start, end - start);
+      if (seg == ".") {
+      } else if (seg == "..") {
+        if (isScope) {
+          return false;
+        }
+        if (!segments.empty()) {
+          segments.pop_back();
+        }
+      } else {
+        segments.emplace_back(seg);
+      }
+    }
+    start = end + 1;
+  }
+  if (segments.empty()) {
+    normalized = "/";
+    return true;
+  }
+
+  for (const auto &seg : segments) {
+    normalized.push_back('/');
+    normalized.append(seg);
+  }


Corrected the wording in the validation function's documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 8 · 3

Open (13)

If TSUrlPathGet() returns nullptr, pathLen may not be reliable; constructing… · New
percentDecodePath() decodes all percent-encoded bytes, including reserved path delimiters like…
std::from_chars support for unsigned char is not consistently available across standard library… · New
normalizePath() allocates a StringVector and builds intermediate strings on every request.… · New
validateScope() collapses multiple failure modes into a single false result (e.g., invalid… · New
This normalization allocates and stores all segments in a vector before rebuilding the normalized…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
This doc line has a grammatical/capitalization issue (. an absent...) and is overly dense for…
normalizePath() allocates/constructs multiple strings per request (decoded plus `StringVector…
Capitalize the sentence start: change 'an absent or empty scope' to 'An absent or empty scope'. · New
The comment reads fails within but should be falls within.
Correct API documentation to say paths fall within scope

Resolved since last review (1)

Typo in the public header comment: fails within should be falls within.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

Review effort: Lite
Findings: 2 · 10 · 4

Open (16)

For request paths (isScope=false), the current normalization resolves .. by popping segments… · New
percentDecodePath() decodes all percent-encoded bytes, including reserved path delimiters like…
Using std::from_chars with unsigned char is implementation-fragile and can fail to compile on… · New
Scope enforcement currently collapses multiple failure modes into OUT_OF_SCOPE: (1) genuinely… · New
validateScope() collapses multiple failure modes into a single false result (e.g., invalid…
normalizePath() allocates a StringVector and builds intermediate strings on every request.…
std::from_chars support for unsigned char is not consistently available across standard library…
This normalization allocates and stores all segments in a vector before rebuilding the normalized…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
This doc line has a grammatical/capitalization issue (. an absent...) and is overly dense for…
normalizePath() allocates/constructs multiple strings per request (decoded plus `StringVector…
Capitalize the start of the sentence after the period: change 'an absent' to 'An absent'. · New
Capitalize the sentence start: change 'an absent or empty scope' to 'An absent or empty scope'.
The comment reads fails within but should be falls within.
Correct API documentation to say paths fall within scope

Resolved since last review (1)

If TSUrlPathGet() returns nullptr, pathLen may not be reliable; constructing…

Corrected capitalization and punctuation in the scope description for clarity. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot

Copilot review overview

🟡 Changes recommended

Critical issues remain with malformed percent-escape handling and decoding reserved path delimiters during authorization.

Review effort: Lite
Findings: 4 · 10

Open (14)

Validate that percent-escape parsing consumes both hex digits · New
Preserve encoded path delimiters during scope authorization · New
For request paths (isScope=false), the current normalization resolves .. by popping segments…
percentDecodePath() decodes all percent-encoded bytes, including reserved path delimiters like…
Scope enforcement currently collapses multiple failure modes into OUT_OF_SCOPE: (1) genuinely…
Using std::from_chars with unsigned char is implementation-fragile and can fail to compile on…
validateScope() collapses multiple failure modes into a single false result (e.g., invalid…
normalizePath() allocates a StringVector and builds intermediate strings on every request.…
std::from_chars support for unsigned char is not consistently available across standard library…
This normalization allocates and stores all segments in a vector before rebuilding the normalized…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
The percent-decoder turns all %xx sequences into bytes, and normalization treats both / and…
This doc line has a grammatical/capitalization issue (. an absent...) and is overly dense for…
normalizePath() allocates/constructs multiple strings per request (decoded plus `StringVector…

Resolved since last review (4)

Capitalize the start of the sentence after the period: change 'an absent' to 'An absent'.
Capitalize the sentence start: change 'an absent or empty scope' to 'An absent or empty scope'.
The comment reads fails within but should be falls within.
Correct API documentation to say paths fall within scope

…ed failure modes # Conflicts: # plugins/experimental/access_control/plugin.cc

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

eveniota added 4 commits September 19, 2026 13:24

scope-validation: Add validateScope() method with path-prefix matching

dccae8a

tests: Add scope validation test cases

6198e2c

Add Catch2 unit tests for validateScope()

scope-validation: Integrate scope check into enforceAccessControl()

eba1cde

docs: Document scope claim behaviour and matching semantics

82a94ad

Copilot AI lite review requested due to automatic review settings September 19, 2026 09:31

Copilot started reviewing on behalf of eveniota September 19, 2026 09:31 View session

Copilot AI reviewed Sep 19, 2026

View reviewed changes

Comment thread doc/admin-guide/plugins/access_control.en.rst Outdated

Comment thread plugins/experimental/access_control/access_control.h Outdated

Update scope description for access control

5bdf062

Clarify scope usage in access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 21, 2026 11:28

Copilot started reviewing on behalf of eveniota September 21, 2026 11:34 View session

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

JosiahWI assigned eveniota Sep 21, 2026

JosiahWI added the access_control label Sep 21, 2026

JosiahWI added this to the 11.0.0 milestone Sep 21, 2026

scope-validation: Canonicalize dot segments and escaped traversal

8c0fddf

Copilot AI review requested due to automatic review settings September 21, 2026 14:54

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Copilot started reviewing on behalf of eveniota September 21, 2026 15:57 View session

scope-validation: Canonicalize path separators and dot

275fb1f

segments

Copilot AI review requested due to automatic review settings September 21, 2026 16:20

Copilot AI reviewed Sep 21, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Copilot started reviewing on behalf of eveniota September 21, 2026 16:54 View session

bryancall requested a review from cmcfarlen September 21, 2026 22:05

Refactor access control path comparison logic

d203a81

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 22, 2026 02:29

Copilot started reviewing on behalf of eveniota September 22, 2026 02:29 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Comment thread plugins/experimental/access_control/access_control.cc Outdated

scope-validation: percent-decoding and reject traversal in scope

62626b9

Copilot AI review requested due to automatic review settings September 22, 2026 17:55

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Update 'scope' parameter description in admin guide

1abf0a3

Clarified the description of the 'scope' parameter in the access control documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 22, 2026 18:38

Copilot started reviewing on behalf of eveniota September 22, 2026 18:53 View session

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Copilot started reviewing on behalf of eveniota September 22, 2026 19:05 View session

Fix documentation wording for access token validation

16b01e3

Corrected the wording in the validation function's documentation. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 22, 2026 19:11

Copilot AI reviewed Sep 22, 2026

View reviewed changes

Copilot started reviewing on behalf of eveniota September 22, 2026 19:34 View session

Handle null path case in access control plugin

2810622

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 23, 2026 07:55

Copilot AI reviewed Sep 23, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Comment thread plugins/experimental/access_control/plugin.cc Outdated

Comment thread doc/admin-guide/plugins/access_control.en.rst Outdated

Copilot started reviewing on behalf of eveniota September 23, 2026 08:38 View session

bryancall added Incompatible New Feature labels Sep 25, 2026

Fix scope description formatting in admin guide

9fea1cd

Corrected capitalization and punctuation in the scope description for clarity. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings September 26, 2026 08:21

Copilot started reviewing on behalf of eveniota September 26, 2026 08:21 View session

Copilot AI reviewed Sep 26, 2026

View reviewed changes

Comment thread plugins/experimental/access_control/access_control.cc Outdated

Comment thread plugins/experimental/access_control/access_control.cc Outdated

scope-validation: Low-allocation single-pass normalization and detail…

35d8e95

…ed failure modes # Conflicts: # plugins/experimental/access_control/plugin.cc

Copilot AI review requested due to automatic review settings September 26, 2026 12:48

Copilot AI reviewed Sep 26, 2026

		while (start < decoded.size()) {
		size_t end = decoded.find_first_of("/\\", start);

Conversation

eveniota commented Sep 19, 2026

Summary

Design Approach (per #13607 suggestion)

Key Requirements & Design Decisions

What Changed

Future Work

Verification

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Copilot review overview

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees