fake_dmserver: sign extension-install policy response [chromium/src : main]
0 views
Skip to first unread message
Nicolas Ouellet-Payeur (Gerrit)
Jan 29, 2026, 1:59:25 PM (2 days ago) Jan 29
to Yann Dago, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Yann Dago
Nicolas Ouellet-Payeur added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Related details
Attention is currently required from:
- Yann Dago
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yann Dago (Gerrit)
Jan 29, 2026, 2:53:13 PM (2 days ago) Jan 29
to Nicolas Ouellet-Payeur, Chromium LUCI CQ, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Nicolas Ouellet-Payeur
Yann Dago voted and added 1 comment![]( "Open in Gerrit")
Votes added by Yann Dago
| Code-Review | +1 |
1 comment
Patchset-level comments
Yann Dago . resolved
looks good, but I am not an expert in what you are doing so maybe we could get another pair of eyes
Related details
Attention is currently required from:
- Nicolas Ouellet-Payeur
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Jan 29, 2026, 2:58:16 PM (2 days ago) Jan 29
to Owen Min, Yann Dago, Chromium LUCI CQ, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Owen Min
Nicolas Ouellet-Payeur added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Owen Min
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Jan 30, 2026, 11:40:38 AM (13 hours ago) Jan 30
to Igor Ruvinov, Yann Dago, Chromium LUCI CQ, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Igor Ruvinov
Nicolas Ouellet-Payeur added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Attention is currently required from:
- Igor Ruvinov
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Igor Ruvinov (Gerrit)
Jan 30, 2026, 1:52:11 PM (11 hours ago) Jan 30
to Nicolas Ouellet-Payeur, Yann Dago, Chromium LUCI CQ, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Attention needed from Nicolas Ouellet-Payeur
Igor Ruvinov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Igor Ruvinov
| Code-Review | +1 |
2 comments
File components/policy/test_support/request_handler_for_policy.cc
Line 413, Patchset 11 (Latest):
em::PolicyData policy_data;Igor Ruvinov . unresolved
Does it matter if there are residual fields in `policy_data` from the for loop after we exit? I see that we overwrite the policy type and value after the loop, just checking that the other fields aren't important.
Line 445, Patchset 11 (Latest):
std::string username = GetUsername(client_info);
std::string domain = gaia::ExtractDomainName(gaia::SanitizeEmail(username));Igor Ruvinov . unresolved
Nit: let's move these down to where they're used, i.e. above `auto* fetch_response = response->add_responses();`
Related details
Attention is currently required from:
- Nicolas Ouellet-Payeur
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Nicolas Ouellet-Payeur (Gerrit)
Jan 30, 2026, 2:00:05 PM (11 hours ago) Jan 30
to Igor Ruvinov, Yann Dago, Chromium LUCI CQ, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Nicolas Ouellet-Payeur added 2 comments![]( "Open in Gerrit")
File components/policy/test_support/request_handler_for_policy.cc
Does it matter if there are residual fields in `policy_data` from the for loop after we exit? I see that we overwrite the policy type and value after the loop, just checking that the other fields aren't important.
Nicolas Ouellet-Payeur
Not really--or at least, I haven't noticed any problems in Chrome and tests still pass.
Keep in mind we don't ship this code to users, so it's not the _end_ of the world if it breaks some weird edge case not covered by tests
Line 445, Patchset 11:
std::string username = GetUsername(client_info);
std::string domain = gaia::ExtractDomainName(gaia::SanitizeEmail(username));Igor Ruvinov . resolved
Nit: let's move these down to where they're used, i.e. above `auto* fetch_response = response->add_responses();`
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Jan 30, 2026, 3:35:48 PM (9 hours ago) Jan 30
to Nicolas Ouellet-Payeur, Igor Ruvinov, Yann Dago, Enterprise Policy Reviews, AyeAye, chromium-a...@chromium.org, extension...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
11 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: components/policy/test_support/request_handler_for_policy.cc
Insertions: 2, Deletions: 2.
@@ -442,8 +442,6 @@
// Wrap the uber-proto with PolicyData and add it to the response.
policy_data.set_policy_type(fetch_request.policy_type());
policy_data.set_policy_value(result.SerializeAsString());
- std::string username = GetUsername(client_info);
- std::string domain = gaia::ExtractDomainName(gaia::SanitizeEmail(username));
if (fetch_request.has_settings_entity_id()) {
policy_data.set_settings_entity_id(
fetch_request.extension_ids_and_version(0).extension_id());
@@ -451,6 +449,8 @@
policy_data.clear_settings_entity_id();
}
+ std::string username = GetUsername(client_info);
+ std::string domain = gaia::ExtractDomainName(gaia::SanitizeEmail(username));
auto* fetch_response = response->add_responses();
fetch_response->set_policy_type(fetch_request.policy_type());
return SerializeAndSignPolicyData(policy_data, fetch_request, domain,
```
Change information
Commit message:
fake_dmserver: sign extension-install policy response
The policy payload isn't passing all the Validator checks, so
make sure the response:
- Includes the username
- Only has settings_entity_id if it's already in the request
- Is properly signed
Bug: 477545526, 468017367
Change-Id: I64f4e7aa827b03b359dbe1c62e5f27ce834ab236
Reviewed-by: Igor Ruvinov <igorr...@chromium.org>
Reviewed-by: Yann Dago <yd...@chromium.org>
Commit-Queue: Nicolas Ouellet-Payeur <nico...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1577477}
Files:
- M components/policy/test_support/request_handler_for_policy.cc
- M components/policy/test_support/request_handler_for_policy.h
Change size: M
Delta: 2 files changed, 77 insertions(+), 42 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Yann Dago, +1 by Igor Ruvinov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages