Skip to content

explicit rules always - #1354

Open
eschultink wants to merge 7 commits into
rc-v0.6.9from
s230-explicit-rules
Open

explicit rules always#1354
eschultink wants to merge 7 commits into
rc-v0.6.9from
s230-explicit-rules

Conversation

@eschultink

@eschultink eschultink commented Jul 23, 2026

Copy link
Copy Markdown
Member

Summary

Prepares Psoxy for hosted proxy deployments where connector specifications live outside the proxy repo—eventually provided by a Worklytics Terraform module (sourced from the main Worklytics codebase) rather than being embedded in worklytics-connector-specs here.

This PR makes rules explicit and externally configurable end-to-end, deprecating reliance on Java prebuilt defaults.

Connector specs → host modules

  • Every connector in worklytics-connector-specs now declares a rules_file pointing at its YAML rules (under docs/sources/).
  • Connector specs pass rules_file paths through to aws-host/gcp-host; the host modules resolve paths (via psoxy_base_dir) and provision RULES to SSM/Secret Manager/env. This is preferred over pre-resolving file content in connector-specs.
  • MSFT connectors use the canonical pseudonymizing *.yaml files; variants that include app/user IDs in the clear are renamed *_inc-app-ids.yaml.

Rules configuration surface (api_connectors)

Two mechanisms, both supported:

Mechanism Use case
rules_file Preferred for self-hosted deployments: path to YAML on disk (resolved by host module)
rules Hosted / external specs: inline YAML content when no local file path is available

rules_raw is deprecated on api_connectors (TODO: remove in v0.7); use rules instead.

Java fallback deprecation

Proxy logs a warning when falling back to prebuilt Java rules (no explicit RULES configured). This fallback will be removed in v0.7.

Test plan

  • terraform test in worklytics-connector-specs
  • terraform validate in infra/examples-dev/aws
  • MSFT rules Java tests (EntraID_NoAppIds_Tests, etc.)
  • ConfigRulesModuleTest

Release notes (0.6.9)

See CHANGELOG.md. MSFT YAML variant filenames changed (*_inc-app-ids.yaml for clear-text app ID variants); default pseudonymizing rules are now the canonical *.yaml files. PSEUDONYMIZE_APP_IDS defaults to true.

aperez-worklytics and others added 2 commits July 22, 2026 12:52
* Connector spec and first test

* Files updated

* Updated files

* Fix link

* Rules update

* Docs and connector spec

* Missing tests

* Rules updated

* Updated test

* Adding more variables

* Updated example and marked as beta

* Rules update

* Rules update

* Adding changelog

* Lint

* Drop versions

---------

Co-authored-by: Erik Schultink <eschultink@users.noreply.github.com>
Wire rules_file for all connectors in worklytics-connector-specs (using no-app-ids variants for MSFT) and log a one-time deprecation warning when the proxy falls back to prebuilt Java rules.

Co-authored-by: Cursor <cursoragent@cursor.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR makes connector sanitization rules selection explicit by referencing YAML rule files in the worklytics-connector-specs Terraform module, and adds a runtime warning when falling back to prebuilt Java rules (to discourage implicit/default behavior).

Changes:

  • Add rules_file references for Microsoft 365, Google Workspace, and multiple OAuth/bulk connectors so rules can be resolved from YAML rather than inline/defaults.
  • Update Terraform tests to reflect that hris now uses rules_file and should produce rules_raw when base_dir is provided.
  • Adjust Java rules-loading fallback to emit a warning when falling back to prebuilt Java rules.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
java/core/src/main/java/co/worklytics/psoxy/ConfigRulesModule.java Adds explicit warning on fallback to prebuilt Java rules and simplifies fallback path.
infra/modules/worklytics-connector-specs/tests/rules_validation.tftest.hcl Updates assertions to match hris switching to rules_file/rules_raw.
infra/modules/worklytics-connector-specs/msft-365.tf Adds rules_file references for Microsoft 365 connectors (defaulting to no-app-ids/no-userIds variants).
infra/modules/worklytics-connector-specs/main.tf Adds rules_file for many connectors and migrates some bulk connectors from inline rules to YAML-backed rules.
infra/modules/worklytics-connector-specs/google-workspace.tf Adds rules_file references for Google Workspace connectors.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +101 to +105
log.warning(String.format(
"No custom RULES found (env var, parameter store, secret manager, GCS, or S3); "
+ "falling back to prebuilt Java rules for SOURCE=%s. "
+ "This fallback will be removed in v0.7; rules must be explicitly configured.",
rulesKey));
eschultink and others added 5 commits July 23, 2026 13:46
Rename pseudonymizing MSFT rules YAML to the base filenames, move clear-text variants to *_inc-app-ids.yaml, and update Terraform, Java, tests, and CHANGELOG.

Co-authored-by: Cursor <cursoragent@cursor.com>
Restore loadAndLog on prebuilt-rules fallback, simplify the deprecation warning, and move 0.6.8 release notes out of the 0.6.9 section.

Co-authored-by: Cursor <cursoragent@cursor.com>
…perty.

Host modules resolve rules paths locally and accept inline rules for hosted deployments, deprecating rules_raw on api_connectors.

Co-authored-by: Cursor <cursoragent@cursor.com>
Avoid coalesce() in the for-expression filter when both rules and
rules_raw are unset, which failed gcp-host concurrency tests.

Co-authored-by: Cursor <cursoragent@cursor.com>
validateYamlExample compares Java prebuilt rules (with app IDs) to
the _inc-app-ids YAML variants after the MSFT canonical rename.

Co-authored-by: Cursor <cursoragent@cursor.com>
Base automatically changed from rc-v0.6.8 to main July 24, 2026 18:50
An error occurred while trying to automatically change base from rc-v0.6.8 to main July 24, 2026 18:50
@eschultink
eschultink changed the base branch from main to rc-v0.6.9 July 24, 2026 19:10
@eschultink eschultink changed the title S230 : explicit rules explicit rules always Jul 28, 2026
@eschultink
eschultink marked this pull request as ready for review July 28, 2026 22:28
- pathTemplate: "/beta/copilot/users/{id}/interactionHistory/getAllEnterpriseInteractions"
allowedQueryParams:
- "$filter"
- pathRegex: "^/beta/copilot/users/(/p~[a-zA-Z0-9_-]+?)?[^/]*/interactionHistory/getAllEnterpriseInteractions(\\\

@jlorper jlorper Jul 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
- pathRegex: "^/beta/copilot/users/(/p~[a-zA-Z0-9_-]+?)?[^/]*/interactionHistory/getAllEnterpriseInteractions(\\\
- pathRegex: "^/beta/copilot/users/(p~[a-zA-Z0-9_-]+?)?[^/]*/interactionHistory/getAllEnterpriseInteractions(\\\

doesn't this have an extra forward slash for the pseudonymized case? /users//p~..../

{
"id": "cdfb873d-61f9-4656-bbbb-a915ef1a255d",
"businessPhones": [
"6175759475"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eschultink @aperez-worklytics again this made it to a PR.

jsonPaths:
- "$..user.id"
- "$..userId"
- "$..emailAddress"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aperez-worklytics emailAddress was removed in the version with ids. there is no emailAddress in that api response I think
https://github.com/Worklytics/psoxy/blob/main/docs/sources/microsoft-365/msft-teams/example-api-responses/original/Teams_v1.0.json

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, that comes because in the code we have that as default for all the endpoints:

static final Transform.Pseudonymize MS_TEAMS_TEAMS_DEFAULT_PSEUDONYMIZE = Transform.Pseudonymize.builder()

in this case, emailAddress is not part of the response payload. At some point we should drop the rules built in code with the ones generated based on our request/response model.

type = map(any)
description = "Map of configuration settings specifically for MSFT 365 connectors (e.g. test GUIDs, custom paths). Note that provider-controlling parameters (like msft_tenant_id or existing app IDs) remain top-level variables."
description = "Map of configuration settings specifically for MSFT 365 connectors (e.g. test GUIDs, custom paths). Supported keys: example_msft_user_guid, example_msft_group_guid, msft_teams_example_team_guid, msft_teams_example_channel_guid, msft_teams_example_chat_guid, msft_teams_example_call_guid, msft_teams_example_call_record_guid, msft_teams_example_online_meeting_join_url, msft_onedrive_example_drive_id, msft_onedrive_example_item_id. Note that provider-controlling parameters (like msft_tenant_id or existing app IDs) remain top-level variables."
default = {}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not a big deal, but asked for a quick validation instead of just listing stuff on the description

Suggested change
default = {}
validation {
condition = length(setsubtract(
toset(keys(var.msft_365_connector_settings)),
toset([
"example_msft_user_guid",
"example_msft_group_guid",
"msft_teams_example_team_guid",
"msft_teams_example_channel_guid",
"msft_teams_example_chat_guid",
"msft_teams_example_call_guid",
"msft_teams_example_call_record_guid",
"msft_teams_example_online_meeting_join_url",
"msft_onedrive_example_drive_id",
"msft_onedrive_example_item_id",
])
)) == 0
error_message = "msft_365_connector_settings contains unsupported keys. Supported keys are: example_msft_user_guid, example_msft_group_guid, msft_teams_example_team_guid, msft_teams_example_channel_guid, msft_teams_example_chat_guid, msft_teams_example_call_guid, msft_teams_example_call_record_guid, msft_teams_example_online_meeting_join_url, msft_onedrive_example_drive_id, and msft_onedrive_example_item_id."
}

.sourceFamily("microsoft-365")
.defaultScopeId("azure-ad")
.sourceKind("outlook-cal")
.rulesFile("outlook-cal_inc-app-ids")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, this feels that the test names should be twisted. Not really in this PR scope, but:

CalendarTests == regular tests pseudonymizing APP IDs. (no suffix, default behavior of the rules of all connectors)

Calendar_NoAppIdTests should be renamed to Calendar_IncludingAppIdsTests.

Same applies to all MSFT test classes

@jlorper jlorper left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two things:

  1. one typo in a rule I believe with an extra forward slash
  2. this branch forked off "old OneDrive PR?", same issue as last week.

The explicit rules part is OK

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants