Update closed trackers if unembargoed #1071

superbuggy · 2025-09-25T17:40:51Z

When closed trackers are unembargoed, with this PR their security levels will be updated instead of being passed over due to being closed.

Closes OSIDB-3729

Jincxz

A suggestion: I think there can be tests written, probably in the endpoints directory, to test that we can un-embargo closed trackers.

JakubFrejlach

I don't think this approach will achieve the desired outcome.

JakubFrejlach · 2025-09-26T12:56:29Z

osidb/serializer.py

+            embargo_status_changed = differ(
+                old_affect.flaw, new_affect.flaw, ["is_embargoed", "unembargo_dt"]
+            )
+            if tracker.is_closed and not embargo_status_changed:


I don't think this is the correct solution for this problem. First, I don't understand why unembargo_dt is taken into account here as it can change multiple times, we are really interested here only about the actual unembargo process. Second, I am looking at the Jira issue description and it states that the only update which should really apply in this case is the security level change (aka unembargo), nothing else. Current implementation will propagate the update to a CLOSED Tracker in Jira during unembargo process but will propagate any changes which would have been made so far between the Tracker becoming CLOSED and the unembargo action.

osidb/serializer.py

superbuggy · 2025-10-01T14:41:06Z

A suggestion: I think there can be tests written, probably in the endpoints directory, to test that we can un-embargo closed trackers.
Good point, I wasn't sure how to test the JIRA/jiraffe portion, so I opted to do a manual test, but maybe there is a way to test the security_level field as well?

Jincxz · 2025-10-01T15:06:09Z

It looks like the JIRA convertor translates the security field to a LDAP group (link to code) which I believe dictates whether the tracker is embargoed. The JIRA tracker can be converted into a OSIDB Tracker and tested as such (example).

superbuggy · 2025-10-02T14:12:52Z

It looks like the JIRA convertor translates the security field to a LDAP group...

Super helpful, thank you. How does it look now?

Jincxz · 2025-10-02T14:26:40Z

I believe that @JakubFrejlach's comment is still applicable. Changes that happen to the Tracker between it being closed and unembargoed might go through.

Example:

Embargoed Tracker is closed
Tracker is edited with change A and the related Flaw is set to be unembargoed
Save occurs and the Tracker becomes unembargoed but also gets change A

Some condition would need to be in place to make sure only the unembargo happens.

JakubFrejlach

As already pointed out by @Jincxz, this still does not fully address the issue requirements.

apps/trackers/jira/save.py

osidb/serializer.py

apps/trackers/jira/save.py

JakubFrejlach

Couple of minor things but othewise LGTM now!

JakubFrejlach · 2025-10-30T14:27:55Z

apps/trackers/jira/query.py


+    def generate_only_security(self):
+        self.generate_base()
+        self._query["fields"]["security"] = None


This is not necessary since you are now generating the base on the previous line, generate_security will eventually se this if necessary.

JakubFrejlach · 2025-10-30T14:28:13Z

apps/trackers/jira/query.py

                self._query["fields"]["security"] = {
                    "name": JIRA_EMBARGO_SECURITY_LEVEL_NAME
                }
+                print(f" QUERY is: {self._query}")


Debug print statement?

JakubFrejlach · 2025-10-30T14:28:38Z

apps/trackers/jira/query.py

+        """
+
+        self.generate_base()
+        self._query["fields"]["security"] = None


This is not necessary since you are now generating the base on the previous line, generate_security will eventually se this if necessary.

Ah yeah I think some stuff crept in from a stash I popped

I think this is still relevant (generate_only_embargo_and_security_level) but otherwise LGTM.

JakubFrejlach · 2025-10-30T14:31:21Z

apps/trackers/jira/save.py

        """
        builder = self.get_builder()
-        query = builder(tracker).query
+        built_query = builder(tracker)


variable contains instantiated builder, not a query, so a think the name is misleading

JakubFrejlach · 2025-10-30T14:35:13Z

osidb/serializer.py

        # no tracker updates for the closed ones
        # because we consider these already done


I would also update this comment so it reflects the current status - no tracker updates for the closed ones with the exception of the security level

JakubFrejlach · 2025-10-30T14:35:20Z

osidb/serializer.py

            # no tracker updates for the closed ones
            # because we consider these already done


I would also update this comment so it reflects the current status - no tracker updates for the closed ones with the exception of the security level

Update changelog Fix logic to exclude undesired behavior Add test Improve test Remove unused imports Update secrets Prevent non-security closed tracker fields form being altered Fix bug Improve organization Improve tests Fix tests and linting Clean up loose ends

superbuggy requested a review from a team September 25, 2025 17:40

Jincxz requested changes Sep 25, 2025

View reviewed changes

JakubFrejlach requested changes Sep 26, 2025

View reviewed changes

superbuggy force-pushed the OSIDB-3729-update-sl-on-closed-tracker-unembargo branch from 0a24716 to a1801d1 Compare October 2, 2025 14:10

superbuggy requested review from JakubFrejlach and Jincxz October 2, 2025 14:10

superbuggy force-pushed the OSIDB-3729-update-sl-on-closed-tracker-unembargo branch from a1801d1 to 3695c7e Compare October 2, 2025 14:12

superbuggy self-assigned this Oct 2, 2025

JakubFrejlach requested changes Oct 3, 2025

View reviewed changes

superbuggy force-pushed the OSIDB-3729-update-sl-on-closed-tracker-unembargo branch 2 times, most recently from 9c6023e to 2b8e92b Compare October 6, 2025 15:40

JakubFrejlach reviewed Oct 15, 2025

View reviewed changes

superbuggy force-pushed the OSIDB-3729-update-sl-on-closed-tracker-unembargo branch 2 times, most recently from 23f82f8 to 6c5c1f6 Compare October 30, 2025 14:00

JakubFrejlach approved these changes Oct 30, 2025

View reviewed changes

superbuggy force-pushed the OSIDB-3729-update-sl-on-closed-tracker-unembargo branch from 6c5c1f6 to c3dbc3d Compare October 30, 2025 15:35

Jincxz approved these changes Oct 30, 2025

View reviewed changes

superbuggy added this pull request to the merge queue Oct 31, 2025

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Oct 31, 2025

		# no tracker updates for the closed ones
		# because we consider these already done

Update closed trackers if unembargoed #1071

Are you sure you want to change the base?

Update closed trackers if unembargoed #1071

Uh oh!

Conversation

superbuggy commented Sep 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Jincxz left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JakubFrejlach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

superbuggy commented Oct 1, 2025

Uh oh!

Jincxz commented Oct 1, 2025

Uh oh!

superbuggy commented Oct 2, 2025

Uh oh!

Jincxz commented Oct 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JakubFrejlach left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JakubFrejlach left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

superbuggy commented Sep 25, 2025 •

edited

Loading

Jincxz left a comment •

edited

Loading

Jincxz commented Oct 2, 2025 •

edited

Loading

JakubFrejlach left a comment •

edited

Loading