Skip to content

fix(prompts): expose only opaque prompt response identifiers - #1517

Closed
seonghobae wants to merge 6 commits into
developfrom
fix/prompt-response-semantic-id
Closed

fix(prompts): expose only opaque prompt response identifiers#1517
seonghobae wants to merge 6 commits into
developfrom
fix/prompt-response-semantic-id

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Owner and bounded context

ContextualWisdomLab/naruon owns the Prompt Catalog response contract. The initial naming repair correctly identified bare PromptResponse.id as underspecified, but fresh independent review found the stronger causal issue: preserving that sequential database identity under an alias still exposed an unnecessary enumerable identifier even though the domain already has opaque prompt_uid.

TDD / direct review repair

  • Regression-first naming commit 2c83f95e04160d75ad348a1eca2d667c8a12313c established the original naming defect.
  • Fresh Devin review on the unchanged original head identified sequential-ID exposure as a valid product/security finding.
  • Follow-up RED contract fb8ca83f7f3010c0be8a3e02484d77318dea1ec9 requires prompt_uid to be the sole response identifier and forbids both id and prompt_record_id from runtime serialization and generated JSON schema, even when the ORM-shaped source still contains private id=17.
  • Production commit 961d6b1685cd22418e40a8910d6c481876cb35d9 removes the database row identity from PromptResponse; ConfigDict(from_attributes=True) continues to extract only the public fields from PromptTemplate.
  • The valid review thread was answered with exact-head evidence and resolved after the source/test/doc repair; the informational alias thread became obsolete and was resolved.

Public contract and security

GET /api/prompts and POST /api/prompts now return the already-present opaque prompt_uid as the sole prompt identifier. The redundant sequential id property is intentionally removed. Organization/workspace scoping and ownership filters remain unchanged; opaque identifiers are defense-in-depth and do not replace object-level authorization.

OWASP API Security Top 10 API1:2023 identifies object identifiers, including sequential integers, as common BOLA attack inputs and recommends unpredictable record identifiers together with authorization checks. The doctoring update records that rationale without claiming that identifier opacity alone provides authorization.

DDD / persistence / research traceability

docs/doctoring/prompt-response-semantic-identifiers.md records the Prompt Catalog bounded context, public-vs-persistence identity boundary, deliberate compatibility tightening, persistence invariants, OWASP API1:2023, and APA 7 traceability to Feitelson et al. (2022) and Alpern et al. (2024). No database schema, persisted row identity, endpoint path, tenant filter, migration, lock, UPSERT, partition, or read/write topology changes.

Exact identity

  • protected base originally inspected: develop@042b0c70531b229af3acbd0421a2f23098d848b3
  • exact current head: 7f9bc105e3c0b7472c693ec30d9d3ae44ed21579
  • branch: fix/prompt-response-semantic-id

Fresh exact-head required checks and qualifying independent non-author current-head approval are mandatory; predecessor/base/model-only evidence does not transfer.

Merge gate

Do not merge until this unchanged current head has all live required checks terminal-success, zero valid unresolved findings/review threads, qualifying independent non-author approval current for the last push, and ordinary branch protection permits it. No force-push, self-approval, administrative bypass, gate weakening, or finding suppression.

Summary by CodeRabbit

  • API Changes

    • Prompt responses now expose only the opaque prompt_uid identifier.
    • Sequential database identifiers are no longer included in API fields or serialized responses.
  • Documentation

    • Added guidance describing the prompt identifier naming and compatibility contract.

devin-ai-integration[bot]

This comment was marked as resolved.

@seonghobae seonghobae changed the title refactor(prompts): make response record identifier explicit fix(prompts): expose only opaque prompt response identifiers Sep 1, 2026

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 0 new potential issues.

Devin Review

@coderabbitai

coderabbitai Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Team

Run ID: c95630ca-6001-4f51-bdcf-95e035d7c2cf

📥 Commits

Reviewing files that changed from the base of the PR and between 042b0c7 and 7f9bc10.

📒 Files selected for processing (3)
  • backend/api/prompts.py
  • backend/tests/test_prompt_response_naming_contract.py
  • docs/doctoring/prompt-response-semantic-identifiers.md

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.


📝 Walkthrough

Walkthrough

PromptResponse now exposes only prompt_uid. Tests verify runtime serialization and JSON schema behavior. A decision record documents the identifier boundary and confirms that persistence and authorization behavior remain unchanged.

Changes

Prompt response identifier contract

Layer / File(s) Summary
Update PromptResponse contract
backend/api/prompts.py
Removes the sequential id field and uses ConfigDict(from_attributes=True).
Verify and document identifier exposure
backend/tests/test_prompt_response_naming_contract.py, docs/doctoring/prompt-response-semantic-identifiers.md
Tests verify model fields, serialized output, and JSON schema. The decision record documents prompt_uid as the sole public identifier and PromptTemplate.id as private persistence data.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 7f9bc

The PR removes sequential database identifiers from prompt responses while preserving existing access controls; no actionable merge-blocking risk remains after normal checks and review.

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: exposing only opaque prompt identifiers in prompt responses.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/prompt-response-semantic-id

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Contributor Author

Superseded after exact semantic comparison with #1206 (goal/carddav-path-traversal-decode@2533f3a5540a48045dddff54b4ca3f8a07fa101b). The canonical PR now contains the same Prompt Catalog behavior: public PromptResponse exposes only opaque prompt_uid, the ORM-shaped/runtime/JSON-schema regression test is byte-identical, and the doctoring retains the same bounded-context, persistence, OWASP API1:2023, and APA-7 rationale. The only remaining source difference in #1517 is the equivalent Pydantic configuration spelling ConfigDict(from_attributes=True) versus the existing {"from_attributes": True} form in #1206; it does not represent a distinct behavior or contract. No unique test or buyer-visible requirement remains here. Closing this duplicate preserves the executable evidence on #1206 rather than maintaining two owners for the same semantic slice.

@seonghobae seonghobae closed this Sep 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant