asf-tooling opened a new issue, #1047:
URL: https://github.com/apache/tooling-trusted-releases/issues/1047
**ASVS Level(s):** L2-only
**Description:**
### Summary
The policy update function uses dynamic field assignment via `setattr()`
loop without an explicit allowlist, creating risk that future model expansions
could inadvertently expose additional writable fields. This violates BOPLA
protection principles where field-level write access should be explicit, not
derived from model structure. Sibling methods (`edit_compose`, `edit_vote`) use
explicit field assignment, creating an inconsistency.
### Details
In `atr/storage/writers/policy.py` lines 117-140, the update function
iterates over `update.model_fields_set` and applies changes via `setattr()`
without checking against an explicit allowlist of editable fields. If the
Pydantic model is expanded with new fields in the future, those fields would
automatically become editable through this endpoint without explicit security
review.
### Recommended Remediation
Define an explicit allowlist and enforce it:
```python
_EDITABLE_POLICY_FIELDS = frozenset({
'manual_vote',
'min_hours',
'github_repository_name',
'github_workflow_path',
# Add other intentionally editable fields
})
def edit_policy(self, policy_id: int, update: PolicyUpdate) -> Outcome:
policy = self._get_policy(policy_id)
# Intersect with allowlist
requested_fields = update.model_fields_set
disallowed_fields = requested_fields - _EDITABLE_POLICY_FIELDS
if disallowed_fields:
return Outcome.err(f"Cannot edit fields: {disallowed_fields}")
for field_name in requested_fields & _EDITABLE_POLICY_FIELDS:
setattr(policy, field_name, getattr(update, field_name))
```
This ensures field editability is explicitly controlled rather than
implicitly derived from the Pydantic model structure.
### Acceptance Criteria
- [ ] `_EDITABLE_POLICY_FIELDS` allowlist defined as frozenset
- [ ] Update function validates requested fields against allowlist
- [ ] Disallowed fields rejected with clear error message
- [ ] Unit tests verify allowlist enforcement
- [ ] Unit tests verify future model additions don't automatically become
editable
- [ ] Documentation updated to reflect explicit field control
### References
- Source reports: L2:8.2.3.md
- Related findings: None
- ASVS sections: 8.2.3
### Priority
Medium
---
---
**Triage notes:** audit_guidance we give room to committees to modify their
policy, and anything that is editable is allowed to be
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
