openai / openai/openai-agents-python

EncryptedSession.pop_item drains recoverable ciphertext when configured with the wrong key

Open
#5,005 3 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
29.6k
Forks
4.8k
Avg merge
1d 20h
Merged PRs (30d)
123

Description

Please read this first
  • Have you read the docs? Yes, the encrypted-session guide and session correction examples.
  • Have you searched for related issues? Yes, searched EncryptedSession and pop_item. This report excludes TTL cleanup, which is covered intentionally by existing tests.
Describe the bug

With an incorrect encryption key, a single EncryptedSession.pop_item() deletes every ciphertext item in the underlying session and returns None. Restoring the correct key afterward cannot recover the conversation. Before the pop, all messages remain readable with the correct key.

A concrete application scenario is an undo/regenerate action after a deployment has loaded the wrong encryption secret. The configuration mistake initially hides history without deleting it (get_items() returns an empty list); one correction action then removes the entire recoverable conversation.

This is not a claim that replacing the key transparently rotates existing ciphertext, or that expired items must remain indefinitely. The reproduction uses freshly written, unexpired items and intentionally demonstrates a configuration error. I am asking whether a destructive operation should distinguish authentication failure from ordinary TTL expiry rather than drain both identically.

Debug information
  • SDK checkout: fbf59a40 (0.22.2).
  • Python: 3.13.14; macOS arm64.
  • Backend: real SQLiteSession in a temporary on-disk database.
  • No model, API key, network, concurrency, timing manipulation, or fabricated backend.
  • Frequency: deterministic.
  • Latest packaged release was not separately installed for this reproduction.
Repro steps
import asyncio
import tempfile
from pathlib import Path
from agents import SQLiteSession
from agents.extensions.memory import EncryptedSession

async def main():
    with tempfile.TemporaryDirectory() as tmp:
        store = SQLiteSession('conversation', str(Path(tmp) / 'history.db'))
        try:
            correct = EncryptedSession(session_id='conversation', underlying_session=store, encryption_key='example-correct-key', ttl=3600)
            wrong = EncryptedSession(session_id='conversation', underlying_session=store, encryption_key='example-wrong-key', ttl=3600)
            messages = [{'role':'user','content':'hello'}, {'role':'assistant','content':'hi'}, {'role':'user','content':'follow-up'}]
            await correct.add_items(messages)
            assert await correct.get_items() == messages
            print('recoverable with correct key before:', len(await correct.get_items()))
            print('wrong-key get_items:', await wrong.get_items())
            print('stored ciphertext after get_items:', len(await store.get_items()))
            print('wrong-key pop_item:', await wrong.pop_item())
            print('stored ciphertext after one pop:', len(await store.get_items()))
            print('recoverable with correct key after:', len(await correct.get_items()))
        finally:
            store.close()

asyncio.run(main())

Observed output:

recoverable with correct key before: 3
wrong-key get_items: []
stored ciphertext after get_items: 3
wrong-key pop_item: None
stored ciphertext after one pop: 0
recoverable with correct key after: 0
Expected behavior / scope clarification

I would expect an incorrect key not to turn a one-item correction into deletion of all recoverable history. An explicit authentication error or a non-destructive refusal would make the configuration problem recoverable. The intended retention policy for this case is not explicitly established in the current docs, so maintainer confirmation of that policy would be useful before choosing a patch.

_unwrap() catches Fernet InvalidToken and returns None for both expiry and authentication failure. pop_item() destructively pops first, then loops whenever _unwrap() returns None (src/agents/extensions/memory/encrypt_session.py:207-216,280-295).

A naive get_items(1) followed by pop_item() is not proposed as a fix: those are separate backend operations and can address different items if another caller mutates the session. A fix should preserve the existing expiry behavior and the underlying pop operation's concurrency guarantees.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/agents/extensions/memory/encrypt_session.py, especially _unwrap() and pop_item() at lines 207-216 and 280-295, then run the provided SQLiteSession reproduction. Confirm the existing TTL behavior and underlying pop concurrency guarantees before deciding how authentication failure differs from expiry; done means a wrong key cannot drain recoverable ciphertext while normal expiry still works.

Written by the indexing model from the issue text.

Assessment

Tech stack
cryptography, python, sqlite
Domain
backend, databases, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.