[2.8] Add class allow-list migration controls

[nvidianz]

What changed

• support "*" in class_allow_list to allow all component classes, ignore remaining entries, and record an audit event
• add class_list_enforcement_mode with enforce (default) and warn behavior
• include the enforcement mode in provisioned client and server resource templates
• document 2.7-to-2.8 migration options in the 2.8 release notes, including unchanged BYOC behavior
• add unit coverage for wildcard, warn/enforce modes, site configuration filtering, and provisioning defaults

Why

NVFLARE 2.8 adds component class authorization for non-BYOC jobs. Applications migrating from 2.7 need a secure default plus explicit transition options while they inventory and configure application classes. BYOC-enabled users and jobs continue to bypass the built-in class allow-list check, preserving their 2.7 behavior.

Validation

• ./runtest.sh -s
• 152 tests passed, 1 skipped across targeted and related unit suites
• git diff --check

The full strict Sphinx build parsed the new release-note content but continues to report 585 pre-existing documentation warnings elsewhere in the repository.

[greptile-apps]

Greptile Summary

This PR adds migration controls for component class authorization in 2.8. The main changes are:

• Wildcard support for class_allow_list.
• New class_list_enforcement_mode with enforce and warn modes.
• Built-in default allow-list fallback and audit logging.
• Provisioning template updates for client and server resources.
• Site config filtering for local authorization policy fields.
• Release notes, security guide updates, and unit coverage.

Confidence Score: 5/5

This looks safe to merge.

• No blocking issues found in the changed code.

Important Files Changed

Filename	Overview
nvflare/app_common/widgets/component_path_authorizer.py	Adds wildcard handling, warn/enforce behavior, default policy fallback, and scoped audit/warning deduplication.
nvflare/app_common/default_component_policy.py	Adds the built-in default component allow-list used when resources omit an explicit list.
nvflare/private/fed/utils/site_config.py	Keeps class_list_enforcement_mode out of exported site configuration.
nvflare/lighter/templates/master_template.yml	Adds explicit enforce mode to provisioned client and server resource templates.

_{Reviews (9): Last reviewed commit: "Clarify class allow-list migration paths" | Re-trigger Greptile}

[chesterxgchen]

Issues and suggestions

1. Audit event is silently dropped when no auditor is initialized, but dedup still marks it as recorded (component_path_authorizer.py, _audit_allow_all).
   AuditService.add_event returns "" and does nothing if the_auditor is None — notably, the simulator never initializes it (simulator_worker.py:256 has the call commented out). The code adds audit_key to _allow_all_audit_keys regardless of whether the event was actually written, so the audit is never retried. The logger.warning still fires once, so there's a log trace, but the PR's stated guarantee ("records an audit event") doesn't hold in simulator runs and in any window before auditor init. Consider checking the return value and only adding to the dedup set on success (falling back to warning-only dedup otherwise), or at least noting the simulator behavior in the docs.

2. Warn mode has no audit trail — inconsistent with the wildcard path.
   Both features "relax the 2.8 protection" (the docs say so explicitly), but only the wildcard records an audit event. In warn mode, each unmatched class that gets loaded is only a site-log warning. From an auditor's perspective these are exactly the events worth recording — a specific unauthorized class was allowed to load. Suggest adding an AuditService.add_event (deduplicated per job + class path) on the warn-mode allow path, mirroring _ALLOW_ALL_AUDIT_ACTION.

3. Audit file I/O performed while holding _allow_all_audit_lock.
   Auditor.add_event does a write + flush inside the lock. Contention is low (dedup means it's rare), so this is minor — but moving the add_event call outside the lock (check-and-mark inside, emit outside) is a small cleanup. Note that would allow a duplicate under a race, which may be an acceptable trade; current code is correct, just holds a lock across file I/O.

4. Redundant file stat on every wildcard-mode component build.
   Under the wildcard, every authorize_component_config call runs _get_policy (stat + cache check) and then _audit_allow_all, which calls _get_resources_file_path again (another os.path.exists/stat) and takes the audit lock even after dedup. Cheap per-call, but it runs once per component per job config parse. Passing the already-resolved resources_file from _get_policy into _audit_allow_all would remove the duplicate resolution.

5. Warn-mode logging bypasses the FL logging convention.
   self.logger.warning(...) works, but FLComponent.log_warning(fl_ctx, msg) would include job/identity context in the log line, which matters here since the whole point of warn mode is inventorying classes per job. fl_ctx is Optional in this path, so it needs a guard, but the event-driven path always has one.

6. Minor test gaps.

• No test that a missing class_allow_list still errors when class_list_enforcement_mode: "warn" is set — the release notes explicitly promise this ("remains a configuration error … in either enforcement mode").
• No test for wildcard loaded via the workspace resources.json file path (the wildcard tests all go through ConfigService), so the 3-tuple cache round-trip for the wildcard/mode combination isn't exercised end-to-end.

7. Docs nit.
   The wildcard example in flare_280.rst shows "class_list_enforcement_mode": "enforce" next to "class_allow_list": [""]. It's technically accurate (mode is irrelevant once the wildcard is present) but slightly confusing — a reader may think enforce is required for the wildcard to audit. A one-line note that the mode has no effect when "" is present would help.

Security Review: PR #4888 — class allow-list migration controls

Threat model context

The control being relaxed exists to protect a site from malicious or compromised jobs: component configs pushed from the server instantiate classes by dotted path, so an unrestricted path is arbitrary code execution on the site (any importable class with dangerous constructor side effects — the tests' own subprocess.Popen example). The right question is: can anyone other than the site admin turn these relaxations on, and what's the blast radius when the admin does?

Findings (by severity)

S1 — Warn mode is site-global and unbounded, with no audit trail (medium).
Enabling warn to migrate one application opens the site to arbitrary class loading by every non-BYOC job from the server, indefinitely — and in FL the server/job-submitter isprecisely the adversary this control defends against. The only record of an unauthorized class actually being loaded is a site log warning; site logs rotate and are not the tamper-evident audit channel. Contrast: the wildcard — the other relaxation — does get an audit event. The event that most warrants auditing ("class X outside the allow-list wasallowed to load for job Y") is the one that isn't recorded. Recommend: emit an AuditService.add_event on every warn-mode allow (dedup per job+class, mirroring the wildcard pattern). Per-job or time-boxed warn scoping would be a worthwhile follow-up, but auditing is the minimum for merge.

S2 — Wildcard audit event can be silently lost while dedup marks it recorded (medium).
AuditService.add_event returns "" and does nothing when no auditor is initialized (never initialized in simulator; simulator_worker.py:256 is commented out) or after the audit file closes. _audit_allow_all adds the dedup key unconditionally, so a dropped event is never retried. The PR's own security guarantee ("records an audit event statingenforcement was bypassed") is best-effort in practice. Recommend: only add to the dedup set when add_event returns a non-empty event id; keep the log warning deduplicated separately.

S3 — Job-controlled strings flow unsanitized into logs that drive security decisions (low-medium).
component_path is validated only as a non-empty str (get_component_path), so a job config can supply a path containing newlines or ANSI escapes. In warn mode that string is interpolated into the warning line admins are told to use to inventory and build their allow list — a malicious job can inject forged log lines (e.g., a fake "not in allow_list"warning for a dangerous class, nudging an admin to whitelist it). Enforce-mode exception messages have the same exposure pre-existing, but warn mode elevates these logs into a decision input, which is why it matters now. Recommend: validate component_path against a dotted-identifier pattern (^[A-Za-z]\w*(.[A-Za-z_]\w*)+$) in _get_component_path —every legitimately importable path matches, and it eliminates the injection class entirely. Same treatment for node_path or log it via repr().

S4 — Audit event omits the source of the wildcard (low).
_audit_allow_all computes source (the resolved resources file path) for the dedup key but doesn't put it in the event's msg or ref. For forensics ("which config file on whichsite had *, and since when"), the event should name the file. One-line fix: append source to msg.

[pcnudde]

I think the first option alone is enough.

[nvidianz]

@chesterxgchen The security-review findings are addressed in 6f8fd535f:

• failed audit writes are retried, while wildcard warning deduplication is tracked separately;
• warn-mode exceptions emit audit events deduplicated per job and class path;
• audit file I/O occurs outside the state lock;
• wildcard handling reuses the already-resolved policy source;
• warning messages use FL-context logging;
• workspace wildcard, retry, source, lock, and cross-job dedup paths have regression coverage;
• component paths are restricted to dotted Python identifiers and node paths are rendered with repr;
• wildcard audit messages include the policy source;
• the release notes clarify that enforcement mode is irrelevant with "*" and document simulator audit behavior.

The suggested missing-list error test was superseded by the accepted behavior that an omitted class_allow_list uses the curated built-in default. The corresponding warn-mode fallback is covered. Validation completed with 150 relevant tests and ./runtest.sh -s.

[nvidianz]

@chesterxgchen Item-by-item follow-up for the review, addressed in 6f8fd535f:

1. Dropped wildcard audit event: audit success and warning deduplication now use separate sets. An audit key is recorded only when AuditService.add_event() returns an event ID, so unavailable audit services are retried on later matching checks while the wildcard warning remains once per job/policy.

2. Warn mode had no audit trail: every unmatched class allowed by warn mode now emits component_authorization.unlisted_class_allowed. Successful audit events are deduplicated per job, policy source, and component path; the site warning is still emitted for every encounter.

3. Audit I/O under the lock: the lock now protects only successful-audit and warning-key checks/updates. AuditService.add_event() runs outside the lock. A regression test asserts that the state lock is not held during the audit call.

4. Redundant resource-file lookup/stat: _get_policy() now returns the resolved policy source together with the list and enforcement mode. Wildcard and warn-mode audit handling reuse that value instead of resolving the workspace resource file again. A test asserts one workspace lookup.

5. Warn logging bypassed FL conventions: warnings now go through FLComponent.log_warning(fl_ctx, ..., fire_event=False) when an FL context is available, preserving job/identity context without generating another log event. Direct calls without an FL context retain the logger fallback.

6. Test gaps: added workspace-file wildcard/cache coverage, failed-audit retry coverage, warn-mode audit dedup coverage, cross-job dedup coverage, source attribution, lock-free audit I/O, and FL-context logging coverage. The proposed “missing list must error in warn mode” test no longer applies because the accepted behavior is for an omitted list to use the curated built-in default; that warn-mode fallback is covered instead.

7. Wildcard documentation: the 2.8 release notes and security guide now state that enforcement mode has no effect when "*" is present. They also document audit retry and simulator warning behavior.

Security findings:

• S1: addressed by the warn-mode audit event described in item 2.
• S2: addressed by retrying unsuccessful audit writes while independently deduplicating the warning, described in item 1.
• S3: component paths must now match a fully qualified dotted Python identifier pattern, and config node paths are rendered with repr in messages to prevent log-line injection.
• S4: wildcard and warn-mode audit messages now include the resolved policy source (absolute resources path when available, otherwise the resources configuration section).

The later simulator P1 is also addressed: audit scope now falls back from RUN_NUM to CURRENT_JOB_ID/JOB_RUN_NUMBER, so distinct simulator jobs do not share a dedup key.

Validation: 150 relevant tests passed and ./runtest.sh -s passed.

[nvidianz]

Detailed update for the latest review feedback:

1. Audit the implicit default policy

   • Added a dedicated audit action: component_authorization.default_class_allow_list_used.
   • The event is emitted whenever class_allow_list is omitted and NVFLARE falls back to DEFAULT_CLASS_ALLOW_LIST.
   • The audit message includes the number of default-listed classes, guidance to configure an explicit site policy, and the resolved policy source (resources or the absolute resources file path).
   • Deduplication uses the existing _add_audit_event_once machinery and is scoped by job/context plus policy source. A successful audit is emitted once for that scope; if audit submission fails, a later authorization attempt can retry.
   • The existing warning remains, but now uses the same scoped once-only reporting path instead of a single process-wide boolean.

2. Preserve explicit-policy behavior

   • An explicitly configured class_allow_list, including an explicit empty list, does not generate the implicit-default audit event.
   • The policy cache now records whether the resolved policy came from the built-in default so cached file policies still report the correct audit decision.

3. Document the security bar for the built-in default

   • Updated unsafe_component_detection.rst to state that every class in DEFAULT_CLASS_ALLOW_LIST must remain safe to import and construct with untrusted, job-controlled arguments.
   • Future additions must be reviewed for constructor side effects and argument-triggered file, process, network, deserialization, or other privileged operations.
   • The migration guide now also states that use of the implicit default is recorded as an audit event.

4. Test coverage

   • Added coverage proving the implicit default is audited and warned once per job/policy scope.
   • Added coverage proving an explicit allow list does not emit the default-policy audit.
   • Extended workspace-file coverage to verify that the audit includes the policy source.
   • Retained the existing drift guard that pins DEFAULT_CLASS_ALLOW_LIST to the provisioned template list.

Validation completed with Python 3.12:

• 152 targeted unit tests passed.
• Full project style check ./runtest.sh -s passed.

The minor informational notes about id() reuse, possible duplicate events under a thread race, inert invalid prefixes, and enum caching were intentionally left unchanged, as requested.

[chesterxgchen]

LGTM

[chesterxgchen]

YT has a fair question.