[DAP-17] TaskConf AAD Part 2A: Mandatory task_info and Interval task validity - #4684
Merged
Conversation
…validity Provisions the task data the HPKE-AAD TaskConfiguration will need (Part 3), with no AAD changes yet. - task_info is now a mandatory, persisted Vec<u8> (non-empty, <=255 bytes) rather than an Option that serde silently dropped. In the DB it's BYTEA NOT NULL; serde/API forms carry it as an unpadded base64url; taskprov passes the real value from its TaskConfiguration. - Task validity is represented as a single Option<Interval> all the way to the DB: the task_end column becomes task_duration, and the API/serde forms are task_start + task_duration. Half-open intervals are now unrepresentable, as we all wanted. Report-admission checks derive start/end from the interval, and just for safety I added that the PATCH task-end path returns 400 when end is < task_start. Part 2B will change url::Url to janus_messages::Url for our in-DB representations, to keep from causing AAD errors with URL normalization business. That turns out to be a nice standalone task. Closes #4625.
jcjones
marked this pull request as ready for review
June 30, 2026 22:44
jcjones
added
the
allow-changed-migrations
Contributor
Author
|
Whoops, how'd I miss that test. Lemme work this out. |
jcjones
marked this pull request as draft
June 30, 2026 23:27
jcjones
marked this pull request as ready for review
June 30, 2026 23:39
Comment on lines
839
to
849
| /// Sets or unsets the end date of a task. | ||
| /// | ||
| /// Because the task's validity interval is stored as a (start, duration) pair and a half-open | ||
| /// interval is not representable, the requested end is translated into a new `task_duration`: | ||
| /// | ||
| /// * `Some(end)`: `task_duration` is recomputed as `end - task_start`, leaving `task_start` | ||
| /// unchanged. If the task has no `task_start`, both columns remain NULL (the interval cannot | ||
| /// be set without a start), which is a no-op. | ||
| /// * `None`: both `task_start` and `task_duration` are cleared, removing the interval entirely. | ||
| #[tracing::instrument(skip(self), err(level = Level::DEBUG))] | ||
| pub async fn update_task_end( |
Collaborator
There was a problem hiding this comment.
This API actually poses a problem now, because it gets incorporated into the task's AAD. Changing the duration midstream would change the AAD, and break decryption of reports. We use this API as the backend for disabling or deleting a task from the control plane currently (not entirely sure which). I think we need to get rid of update_task_end() and introduce some new implementation-specific task field that we can modify to reimplement this feature.
Contributor
Author
There was a problem hiding this comment.
I'm playing with adding a deactivate_at time outside the task configuration, as an Option that we can set via API and it just ... deactivates then. If we set it to now or the past, it's instant.
Contributor
Author
There was a problem hiding this comment.
@divergentdave please take a look at aacea12 and see what you think!
Co-authored-by: David Cook <divergentdave@gmail.com>
When we get to part 3, the task validity interval is in the HPKE-AAD TaskConfiguration, so it must be immutable: mutating it would change the AAD and break decryption of already-uploaded reports. Remove update_task_end() and the PATCH task-end path. Task disable/expire now uses a new deactivate_at column that is not part of the TaskConfiguration, so it can change freely without affecting AADs. NOTE: open a divviup-api issue to switch task disabling to use deactivate_at.
divergentdave
approved these changes
Jul 1, 2026
Co-authored-by: David Cook <dcook@divviup.org>
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Provisions the task data the HPKE-AAD TaskConfiguration will need (Part 3), with no AAD changes yet.
task_info is now a mandatory, persisted
Vec<u8>(non-empty, <=255 bytes) rather than anOptionthat serde silently dropped. In the DB it'sBYTEA NOT NULL; serde/API forms carry it as an unpadded base64url; taskprov passes the real value from itsTaskConfiguration.Task validity is represented as a single
Option<Interval>all the way to the DB: thetask_endcolumn becomestask_duration,and the API/serde forms aretask_start + task_duration. Half-open intervals are now unrepresentable, as we all wanted. Report-admission checks derive start/end from the interval, and just for safety I added that thePATCHtask-end path returns 400 when end is< task_start.Part 2B will change
url::Urltojanus_messages::Urlfor our in-DB representations, to keep from causing AAD errors with URL normalization business. That turns out to be a nice standalone task.Closes #4625.