Repository navigation
security: SecurityPolicyManager and SSOIntegrationFramework read policy files that have never existed #15154
Description
Activity
Flagging as needs-decision — this genuinely needs an owner call per the issue's own text ("Deciding what the reviewed policy should say is a security decision, not a mechanical one"), not something to resolve unilaterally.
Options, with a recommendation:
-
SecurityPolicyManager(security_policies.yaml): enforcement mode, violation threshold, compliance framework list are real, universally-applicable security posture — not tied to any specific enterprise integration. Recommend: materialize the file with the current code defaults as the starting reviewed policy (they're not placeholder-shaped, unlike SSO's), then adjust from there if the owner wants different values. This is a one-time "promote the default to reviewed config" action, not new policy design. -
SSOIntegrationFramework(sso_config.yaml): its defaults contain unambiguous placeholders (autobot-enterpriseentity id, a placeholder service URL,dc=company,dc=comsearch bases) — these read as example/template values, not real config. Given AutoBot's own framing ("self-hosted, agentic AI you own"), recommend: the built-in default is likely the intended posture here (SSO probably isn't actually deployed against a real IdP for most installs) — say so explicitly at the call site (sso_integration.py:95) rather than naming a path that will never resolve, and drop the placeholder values from the default entirely rather than let them look like configured policy.
Either way, whoever decides should also weigh in on whether
_KNOWN_ABSENT_CONFIG_FILESinrepo_tests/config_dir_resolves_14892_test.pyshould drop the SecurityPolicyManager entry (if materialized) while keeping the SSO one (if the built-in-default path is chosen for that one).-
The facts behind the decision this issue asks for — and they change the question
Every criterion here opens with "Decide, per manager…", so this is an owner call rather than work
anyone can just do. Establishing the current state againstorigin/mainso the decision is one step.Neither manager is wired
git grepfor both class names across the whole tree returns four files, and not one is a call site:security/enterprise/__init__.py:12,13,20,21 import + __all__ re-export security/enterprise/security_policy_manager.py:105 class definition security/enterprise/sso_integration.py:88 class definition repo_tests/config_dir_resolves_14892_test.py:130 a comment about themNothing instantiates
SecurityPolicyManagerorSSOIntegrationFramework. So "reads policy files
that have never existed" is a symptom: the files were never created because the classes that would read
them were never wired in.The missing-file half is already handled
load_config_file(autobot_shared/config_file_loading.py:63) is documented as falling back "loudly
and without writing" — it logs a warning naming the path and the description, then uses the built-in
defaults. So a missingsecurity_policies.yamlorsso_config.yamlis no longer a silent failure,
whatever is decided here. Confirmed neither file exists in the repository.The placeholders are real but unreachable
_get_default_ldap_config()(sso_integration.py:170) shipsou=users,dc=company,dc=comand
ou=groups,dc=company,dc=com. Those are AC4's "placeholder SAML/LDAP values". They are template
strings rather than credentials, and with no call site nothing reads them — but they would become live
defaults the moment someone wired the framework up, which is the more dangerous shape: a plausible
config that was never reviewed, inherited by adoption.So the decision is narrower than four options
The issue offers "the file should exist" versus "the built-in default is intended". Neither fits an
unwired class. The real question is the project's standing one for dead code — wire it in, or retire
it — and the answer determines the rest:- Wire it in → the config files must exist with reviewed values before the first call site lands,
because a wired framework reading unreviewed defaults is worse than an unwired one. - Retire it → the placeholders and both config paths go with it, and AC4 resolves by deletion.
Recording rather than choosing: this is an architecture call, and "the class is unwired" is the fact
that was missing from the issue when it was written.Verified during the v0.9.0 closure pass.
- Wire it in → the config files must exist with reviewed values before the first call site lands,
Problem
Two of the five enterprise-security managers default their
config_pathto afile that no commit on any branch has ever added:
SecurityPolicyManager(security_policy_manager.py:112)PATH.get_config_path("security", "security_policies.yaml")SSOIntegrationFramework(sso_integration.py:95)PATH.get_config_path("security", "sso_config.yaml")ComplianceManagersecurity/compliance.yamlDomainReputationServicesecurity/domain_security.yamlThreatDetectionEnginesecurity/threat_detection.yamlVerified with
git log --all -- "**/security_policies.yaml" "**/sso_config.yaml"— empty.So the security policy actually in force, and the SSO posture actually in force,
have only ever been each manager's
_get_default_config()— a code defaultnobody reviewed as policy. Both defaults are non-trivial: enforcement mode
enforce, a violation threshold, a compliance framework list,sso_enabled: true,auto_provision_users: true,require_group_membership: false.This contradicts #14892's own problem statement, which listed all five files as
present. Three are; two are not.
What #14892 already did
#14892 made the miss distinguishable rather than silent: both managers now
expose
config_sourceand log a WARNING naming the exact path searched, and themiss branch no longer writes its defaults back to that path. So the condition is
now observable. It is not resolved.
The absence is pinned in
repo_tests/config_dir_resolves_14892_test.py(
_KNOWN_ABSENT_CONFIG_FILES) as a two-entry ratchet, so a third absent filefails the guard and removing either of these two without deleting its entry also
fails.
Why this is not a refactor
Deciding what the reviewed policy should say is a security decision, not a
mechanical one. Materialising the current defaults verbatim would be worse than
leaving them in code: the SSO defaults contain placeholder values
(
autobot-enterpriseentity id, a placeholder service URL,dc=company,dc=comsearch bases) that would then read as reviewed configuration.
Acceptance criteria
default is the intended policy
remove its entry from
_KNOWN_ABSENT_CONFIG_FILESnaming a path that will never resolve
shipped as policy
Related