asf-tooling commented on issue #708:
URL:
https://github.com/apache/tooling-trusted-releases/issues/708#issuecomment-4410102548
<!-- gofannon-issue-triage-bot v2 -->
**Automated triage** — analyzed at `main@2da7807a`
**Type:** `new_feature` • **Classification:** `actionable` •
**Confidence:** `medium`
**Application domain(s):** `cryptographic_keys`
### Summary
Issue #708 requests a 'historical only' flag for OpenPGP keys that were
previously used to sign releases but are no longer valid for current use. The
flag would preserve these keys for record-keeping while excluding them from
active KEYS file generation. @dave2wave noted this should be implemented
alongside #911. Currently no such flag exists on the `PublicSigningKey` model.
The feature requires a new database column, UI to toggle it, and logic to
filter historical keys from active KEYS files.
### Where this lives in the code today
#### `atr/storage/writers/keys.py` — `FoundationCommitter.public_key_model`
(lines 235-261)
_extension point_
This is where PublicSigningKey models are constructed; a new `historical`
field would default to False here.
```python
def public_key_model(
self,
key: openpgp.PublicKey,
ldap_data: dict[str, str],
original_key_block: str | None = None,
) -> sql.PublicSigningKey:
uids = list(key.user_ids)
asf_uid = self.__uids_asf_uid(uids, ldap_data)
if not uids:
raise ValueError("No UIDs found in key")
# Use the original key block if available
ascii_armored = original_key_block if original_key_block else
key.to_armored()
expires_at = _key_expires_at(key)
return sql.PublicSigningKey(
fingerprint=key.fingerprint.lower(),
algorithm=_algorithm_id(key.public_key_algorithm),
length=_key_length(key),
created=datetime.datetime.fromtimestamp(key.created_at,
tz=datetime.UTC),
latest_self_signature=_latest_self_signature_created_at(key),
expires=expires_at,
primary_declared_uid=uids[0],
secondary_declared_uids=uids[1:],
apache_uid=asf_uid,
ascii_armored_key=ascii_armored,
)
```
#### `atr/storage/writers/keys.py` — `FoundationCommitter._keys_file_text`
(lines 269-287)
_needs modification_
KEYS file generation iterates all committee keys; needs filtering to exclude
historical-only keys from active KEYS files.
```python
async def _keys_file_text(self, committee: sql.Committee) -> str:
if not committee.public_signing_keys:
raise storage.AccessError(f"No keys found for committee
{committee.key} to generate KEYS file.", status=404)
sorted_keys = sorted(committee.public_signing_keys, key=lambda k:
k.fingerprint)
keys_content_list = []
for key in sorted_keys:
apache_uid = key.apache_uid.lower() if key.apache_uid else None
# TODO: What if there is no email?
email = util.email_from_uid(key.primary_declared_uid or "") or ""
comments = []
comments.append(f"Comment: {key.fingerprint.upper()}")
if (apache_uid is None) or (email == f"{apache_uid}@apache.org"):
comments.append(f"Comment: {email}")
else:
comments.append(f"Comment: {email} ({apache_uid})")
comment_lines = "\n".join(comments)
armored_key = key.ascii_armored_key
```
#### `atr/shared/keys.py` — `UpdateKeyCommitteesForm` (lines 105-109)
_extension point_
This form handles key detail updates; the historical flag toggle could be
added here or as a separate form.
```python
class UpdateKeyCommitteesForm(form.Form):
selected_committees: form.StrList = form.label(
"Associated PMCs",
widget=form.Widget.CUSTOM,
)
```
### Where new code would go
- `atr/models/sql.py` — in PublicSigningKey model class
Add a `historical: bool = False` column to the PublicSigningKey SQLModel
to persist the flag.
- `atr/storage/writers/keys.py` — after
FoundationCommitter.update_committee_associations
Add a method like `mark_key_historical(fingerprint, historical: bool)` to
toggle the flag.
- `atr/post/keys.py` — after details function
Add a POST handler for toggling the historical flag on a key.
### Proposed approach
The implementation requires adding a `historical` boolean column (defaulting
to `False`) to the `PublicSigningKey` model in `atr/models/sql.py`. This flag
indicates that a key is retained for record-keeping only and should not be
included in actively-generated KEYS files.
The `_keys_file_text` method in `atr/storage/writers/keys.py` should filter
out keys where `historical=True` when generating committee KEYS files (or
include them in a clearly-separated 'historical' section). A new writer method
`mark_key_historical` should allow toggling the flag. The key details page
(`atr/get/keys.py`) should display the historical status and provide a toggle
for the key owner. As @dave2wave suggested, this should be coordinated with
issue #911 for a cohesive implementation.
### Suggested patches
#### `atr/models/sql.py`
Add historical flag to PublicSigningKey model (exact location depends on
model definition, which is not in the provided files)
````diff
--- a/atr/models/sql.py
+++ b/atr/models/sql.py
@@ # In the PublicSigningKey class definition, add:
+ historical: bool = sqlmodel.Field(default=False, description="Key is
retained for historical record only and should not be used for current signing")
````
#### `atr/storage/writers/keys.py`
Filter out historical keys from active KEYS file generation
````diff
--- a/atr/storage/writers/keys.py
+++ b/atr/storage/writers/keys.py
@@ -229,7 +229,8 @@ class FoundationCommitter(GeneralPublic):
async def _keys_file_text(self, committee: sql.Committee) -> str:
if not committee.public_signing_keys:
raise storage.AccessError(f"No keys found for committee
{committee.key} to generate KEYS file.", status=404)
- sorted_keys = sorted(committee.public_signing_keys, key=lambda k:
k.fingerprint)
+ active_keys = [k for k in committee.public_signing_keys if not
k.historical]
+ sorted_keys = sorted(active_keys, key=lambda k: k.fingerprint)
keys_content_list = []
for key in sorted_keys:
````
#### `atr/storage/writers/keys.py`
Add method to toggle historical flag on a key
````diff
--- a/atr/storage/writers/keys.py
+++ b/atr/storage/writers/keys.py
@@ # After update_committee_associations method in FoundationCommitter class:
+ async def set_key_historical(self, fingerprint: str, historical: bool)
-> outcome.Outcome[sql.PublicSigningKey]:
+ """Mark a key as historical-only (not for current use) or restore
it to active status."""
+ try:
+ key = await self.__data.public_signing_key(
+ fingerprint=fingerprint,
+ apache_uid=self.__asf_uid,
+ _committees=True,
+ ).demand(storage.AccessError(f"Key not found: {fingerprint}",
status=404))
+ key.historical = historical
+ await self.__data.commit()
+ self.__write_as.append_to_audit_log(
+ action="key_set_historical",
+ asf_uid=self.__asf_uid,
+ fingerprint=key.fingerprint,
+ historical=historical,
+ )
+ # Regenerate KEYS files for affected committees
+ affected_committee_keys = {committee.key for committee in
key.committees}
+ for committee_key in sorted(affected_committee_keys):
+ await self._sync_committee_keys_file(committee_key)
+ return outcome.Result(key)
+ except Exception as e:
+ return outcome.Error(e)
````
### Open questions
- What is issue #911 that @dave2wave referenced? The implementation should
be coordinated with that issue.
- Should historical keys still appear in KEYS files in a separate section
(e.g., for verification of old releases), or be excluded entirely?
- The exact location of the `PublicSigningKey` model in `atr/models/sql.py`
is not visible in the provided files - the diff for that file needs to be
anchored to the actual model definition.
- Should there be an automatic mechanism to mark keys as historical when
they expire, or should this always be a manual action?
- Should PMC members (not just key owners) be able to mark keys as
historical for their committee?
### Files examined
- `atr/storage/writers/keys.py`
- `atr/storage/writers/ssh.py`
- `atr/post/keys.py`
- `atr/get/keys.py`
- `atr/shared/keys.py`
- `atr/tasks/keys.py`
---
*Draft from a triage agent. A human reviewer should validate before merging
any change. The agent did not run tests or verify diffs apply.*
--
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]
