apps/secrets/services/revision.py: only preserve OTP fields if they existed previously

avoid adding explicit nulls, those would change the hash and create
duplicate revisions for unchanged secrets

Author: nkoch-seibert

teamvault/apps/secrets/services/revision.py

@@ -202,9 +202,15 @@ def _build_revision(
         # merge missing fields for PASSWORD type
         if content_type == ContentType.PASSWORD and secret.current_revision:
             prev = secret.current_revision.peek_data(actor)
-            payload.setdefault('password', prev.get('password'))
-            for fld in ('otp_key', 'digits', 'algorithm'):
-                payload.setdefault(fld, prev.get(fld))
+            if 'password' not in payload and 'password' in prev:
+                payload['password'] = prev['password']
+
+            # Preserve OTP fields only when the previous revision actually had them.
+            if 'otp_key' not in payload and 'otp_key' in prev:
+                payload['otp_key'] = prev['otp_key']
+                for fld in ('digits', 'algorithm'):
+                    if fld in prev:
+                        payload[fld] = prev[fld]
 
         sha_src = dumps(payload, sort_keys=True)
         sha_sum = sha256(sha_src.encode()).hexdigest()

teamvault/apps/secrets/tests/models/test_secret_crud.py

@@ -150,6 +150,42 @@ def test_update_file_payload_roundtrip(self):
         self.assertIsInstance(got, (bytes, bytearray))
         self.assertEqual(got, raw)
 
+    def test_otp_only_update_creates_new_revision(self):
+        s: Secret = new_secret(self.owner, name="otp-add")
+        rev_before = s.current_revision
+        self.assertFalse(rev_before.otp_key_set)
+
+        changes_before = SecretChange.objects.filter(secret=s).count()
+
+        # OTP-only update: omit password, keep existing via merge logic.
+        RevisionService.save_payload(
+            secret=s,
+            actor=self.owner,
+            payload={
+                "otp_key": "JBSWY3DPEHPK3PXP",
+                "digits": "6",
+                "algorithm": "SHA1",
+            },
+        )
+
+        s.refresh_from_db()
+        rev_after = s.current_revision
+        self.assertNotEqual(rev_after.id, rev_before.id)
+        self.assertTrue(rev_after.otp_key_set)
+        self.assertEqual(rev_after.get_data(self.owner)["otp_key"], "JBSWY3DPEHPK3PXP")
+
+        # Re-saving identical OTP-only payload should be a no-op.
+        RevisionService.save_payload(
+            secret=s,
+            actor=self.owner,
+            payload={
+                "otp_key": "JBSWY3DPEHPK3PXP",
+                "digits": "6",
+                "algorithm": "SHA1",
+            },
+        )
+        self.assertEqual(SecretChange.objects.filter(secret=s).count(), changes_before + 1)
+
     def test_delete_marks_secret_and_denies_visibility_and_read(self):
         s: Secret = new_secret(self.owner, name="todelete", access_policy=AccessPolicy.ANY)
         s.status = SecretStatus.DELETED