From 88acb5312a2706a64dfc75f1e97cffe3bb70a1eb Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Sat, 3 Oct 2026 00:16:41 -0400 Subject: [PATCH 1/3] Limit content screening intake to uploads and apply metadata edits directly Layer 2 of 4 for #1621, stacked on #1628. - Mark documents SimpleChat generates (agent uploads and published artifacts) with a server-managed screening_exemption field set in create_document before the initial screening marker is chosen. Exempt documents without a marker are never enrolled, prepared, scanned or picked up by scan jobs. User uploads, chat uploads saved to a workspace and File Sync are still screened. - Remove the publication screening reservation and its consume latch. Approval admits a destination without a screening marker or with an available release. - Apply metadata edits and model-generated metadata directly instead of queueing a metadata rescan, keep the source-format rename rule, stop comparing the metadata fingerprint in release proof and recovery, and stop rewriting a screened release blob's metadata on tag edits. - Bump VERSION to 0.261.230 and update release notes, framework, guide and API docs; add the upload-only intake fix document and tests. Refs #1621 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../single_app/content_screening/access.py | 4 +- .../single_app/content_screening/contracts.py | 30 + .../single_app/content_screening/jobs.py | 17 +- .../single_app/content_screening/service.py | 54 +- .../functions_artifact_publication.py | 180 +---- ...unctions_artifact_publication_readiness.py | 66 +- application/single_app/functions_documents.py | 56 +- .../functions_group_document_management.py | 6 +- .../functions_group_document_publication.py | 50 +- .../functions_public_document_management.py | 6 +- .../functions_public_document_publication.py | 59 +- .../functions_simplechat_operations.py | 3 + .../features/CONTENT_SCREENING_FRAMEWORK.md | 16 +- .../features/DOCUMENT_PROVENANCE.md | 13 +- .../GROUP_DOCUMENT_COLLABORATION_APIS.md | 77 +- .../GROUP_DOCUMENT_MANAGEMENT_APIS.md | 8 +- .../PUBLIC_DOCUMENT_ARTIFACT_APPROVAL_APIS.md | 29 +- .../PUBLIC_DOCUMENT_MANAGEMENT_APIS.md | 4 +- .../fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md | 108 +++ docs/explanation/release_notes.md | 17 + docs/guides/review-screened-documents.md | 14 +- .../test_content_screening_jobs.py | 109 +-- .../test_content_screening_pipeline.py | 39 +- ...st_content_screening_upload_only_intake.py | 702 +++++++++++++++++ .../test_group_document_management.py | 51 +- ...ocument_publication_screening_bootstrap.py | 734 ------------------ .../test_public_document_publication.py | 98 +-- 28 files changed, 1200 insertions(+), 1352 deletions(-) create mode 100644 docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md create mode 100644 functional_tests/test_content_screening_upload_only_intake.py delete mode 100644 functional_tests/test_group_document_publication_screening_bootstrap.py diff --git a/application/single_app/config.py b/application/single_app/config.py index 40772cb69..1bfe253f0 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -101,7 +101,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.261.229" +VERSION = "0.261.230" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/content_screening/access.py b/application/single_app/content_screening/access.py index 7f8e8a237..f34c89d15 100644 --- a/application/single_app/content_screening/access.py +++ b/application/single_app/content_screening/access.py @@ -56,7 +56,6 @@ HELD_STATES, content_fingerprint, hash_payload, - metadata_fingerprint, normalize_units, public_screening_summary, require_document_available, @@ -321,7 +320,7 @@ def _read_authorized_document( def _require_release_proof(document, container): - """A restored marker or stale metadata write cannot create a clearance.""" + """A restored marker cannot create a clearance; later metadata edits do not revoke one.""" _require_available_metadata(document) marker = document[SCREENING_FIELD] try: @@ -346,7 +345,6 @@ def _require_release_proof(document, container): or not isinstance(publication, Mapping) or publication.get("active_blob") != marker.get("active_blob") or publication.get("content_fingerprint") != marker.get("content_fingerprint") - or publication.get("metadata_fingerprint") != metadata_fingerprint(document) ): raise DocumentHeldError() diff --git a/application/single_app/content_screening/contracts.py b/application/single_app/content_screening/contracts.py index 2bfa89610..803e63089 100644 --- a/application/single_app/content_screening/contracts.py +++ b/application/single_app/content_screening/contracts.py @@ -9,6 +9,10 @@ SCHEMA_VERSION = 1 SCREENING_FIELD = "content_screening" +# Server-managed marker for workspace documents that SimpleChat generated. The +# "screening" prefix keeps it out of client payloads in both directions. +SCREENING_EXEMPTION_FIELD = "screening_exemption" +GENERATED_SCREENING_EXEMPTION = {"reason": "generated", "version": 1} SCOPE_TYPES = frozenset({"personal", "group", "public"}) AVAILABLE_STATES = frozenset({"cleared", "approved_with_flags"}) HELD_STATES = frozenset({ @@ -86,6 +90,13 @@ class ScreeningConflictError(ScreeningError): public_message = "The document or review changed. Refresh it before trying again." +class ScreeningNotRequiredError(ScreeningConflictError): + """A generated document is never enrolled, so scan jobs skip it.""" + + code = "screening_not_required" + public_message = "This document was generated by SimpleChat and is not screened." + + class DocumentHeldError(ScreeningError): code = "document_under_review" status_code = 409 @@ -126,6 +137,25 @@ def metadata_fingerprint(document: Mapping[str, Any]) -> str: return hash_payload({name: document.get(name) for name in CONTENT_METADATA_FIELDS}) +def generated_screening_exemption() -> dict: + """Return the marker that server code stores on a generated workspace document.""" + return dict(GENERATED_SCREENING_EXEMPTION) + + +def is_generated_screening_exempt(document: Mapping[str, Any]) -> bool: + """Only the exact server-written marker exempts a document; anything else stays screened.""" + if not isinstance(document, Mapping): + return False + exemption = document.get(SCREENING_EXEMPTION_FIELD) + return ( + isinstance(exemption, Mapping) + and set(exemption) == set(GENERATED_SCREENING_EXEMPTION) + and exemption.get("reason") == GENERATED_SCREENING_EXEMPTION["reason"] + and type(exemption.get("version")) is int + and exemption["version"] == GENERATED_SCREENING_EXEMPTION["version"] + ) + + def normalize_identifier(value: Any, field_name: str = "identifier") -> str: if not isinstance(value, (str, int)) or isinstance(value, bool): raise ScreeningValidationError(f"A valid {field_name} is required.") diff --git a/application/single_app/content_screening/jobs.py b/application/single_app/content_screening/jobs.py index 708090bc0..104e3ca54 100644 --- a/application/single_app/content_screening/jobs.py +++ b/application/single_app/content_screening/jobs.py @@ -24,13 +24,14 @@ ScreeningConfigurationError, ScreeningConflictError, ScreeningError, + ScreeningNotRequiredError, ScreeningPolicyRequiredError, ScreeningValidationError, Subject, content_fingerprint, document_is_available, hash_payload, - metadata_fingerprint, + is_generated_screening_exempt, normalize_identifier, normalize_units, subject_from_document, @@ -397,7 +398,7 @@ def _verify_ingestion_job(job, subject, scan_id, actor_id): def enqueue_document_scan(subject, actor_id, *, scan_id=None, repository=None): """Idempotently enroll an already-authorized ingestion/reprocessing scan. - Internal upload/metadata adapters must stage and authorize the exact scan + Internal upload/reprocessing adapters must stage and authorize the exact scan before calling this helper. It never grants workspace or evidence access, and must not be exposed as an alternative to role-checked create_scan_job. """ @@ -825,7 +826,6 @@ def _publication_matches_document(scan, document): and scan["units_ref"] == marker.get("canonical_ref") and scan.get("content_fingerprint") == marker.get("content_fingerprint") == publication.get("content_fingerprint") and scan.get("policy_fingerprint") == marker.get("policy_fingerprint") == hash_payload(scan.get("policy")) - and publication.get("metadata_fingerprint") == metadata_fingerprint(document) and publication.get("active_blob") == marker.get("active_blob") == active_blob and bool(scan.get("sanitized")) == bool(marker.get("sanitized")) ) @@ -886,6 +886,8 @@ def _hold_document(repository, job, item, owner, *, use_service=False): if expires_at is not None: raise _ScanLeaseBusy(expires_at) document = repository.read_document(subject) + if SCREENING_FIELD not in document and is_generated_screening_exempt(document): + raise ScreeningNotRequiredError() marker = document.get(SCREENING_FIELD, {}) if ( not isinstance(marker, dict) @@ -1162,19 +1164,12 @@ def _reconcile_scan_completion(repository, scan_id): if not _completed_scan(repository, {"scan_id": scan_id, "subject": scan["subject"]}): document = repository.read_document(Subject.from_dict(scan["subject"])) marker = document.get(SCREENING_FIELD) - successor = repository.get_scan(marker["scan_id"]) if isinstance(marker, dict) and marker.get("scan_id") else None inactive_review = ( scan["state"] == "pending_review" and isinstance(marker, dict) and marker.get("scan_id") and marker["scan_id"] != scan_id ) - if not inactive_review and ( - scan["state"] not in AVAILABLE_STATES or not successor - or successor.get("subject") != scan["subject"] or successor.get("trigger") != "metadata" - or (successor.get("previous_marker") or {}).get("scan_id") != scan_id - ): + if not inactive_review: return False - # A crash may leave the completed parent's acknowledgement pending - # after metadata has already queued a new, independently held scan. if repository.count("work_item", filters={ "scan_id": scan_id, "status": ["queued", "running", "retry"], }): diff --git a/application/single_app/content_screening/service.py b/application/single_app/content_screening/service.py index 5d66393d0..7cf8c93e9 100644 --- a/application/single_app/content_screening/service.py +++ b/application/single_app/content_screening/service.py @@ -26,11 +26,13 @@ ScreeningConfigurationError, ScreeningConflictError, ScreeningError, + ScreeningNotRequiredError, ScreeningPolicyRequiredError, ScreeningValidationError, Subject, content_fingerprint, hash_payload, + is_generated_screening_exempt, metadata_fingerprint, normalize_units, subject_from_document, @@ -138,11 +140,16 @@ def get_effective_policy(subject, *, repository=None, require_active=True): def document_requires_screening(document, settings=None, *, repository=None): - """Persisted enrollment always wins over the absence of checks for new uploads.""" + """Persisted enrollment always wins over the absence of checks for new uploads. + + A generated document carries a server-written exemption and is never enrolled. + """ if not isinstance(document, dict): raise ScreeningValidationError("The document metadata is unavailable.") if SCREENING_FIELD in document: return True + if is_generated_screening_exempt(document): + return False settings = _settings(settings) if ( settings.get("enable_content_screening") is not True @@ -229,19 +236,12 @@ def _set_document_state(repository, scan, state, **updates): ) -def _consume_publication_reservation(document, scan_id): - if document.get("group_id") and document.get("generated_artifact_publication_receipt_id"): - # Publication imports ingestion; resolve the receipt owner only at scan - # execution, never during service/configuration bootstrap. - from functions_artifact_publication import consume_artifact_publication_screening_scan - - consume_artifact_publication_screening_scan(document, scan_id) - - def begin_scan(subject, actor_id, *, repository=None, storage=None, job_id=None, scan_id=None, expected_document_etag=None, parent_scan_id=None): repository = _repository(repository) document = repository.read_document(subject) + if SCREENING_FIELD not in document and is_generated_screening_exempt(document): + raise ScreeningNotRequiredError() settings = _settings() if settings.get("enable_content_screening") is not True: raise ScreeningConfigurationError("Enable content screening before rescanning inspected content.") @@ -257,7 +257,6 @@ def begin_scan(subject, actor_id, *, repository=None, storage=None, job_id=None, ): raise ScreeningConflictError() scan_id = scan_id or uuid.uuid4().hex - _consume_publication_reservation(document, scan_id) existing = repository.get_scan(scan_id) if existing: if ( @@ -547,7 +546,6 @@ def inspect_scan(scan_id, units, actor_id, *, repository=None, storage=None, eng units = normalize_units(units) scan = _read_scan(repository, scan_id) subject = Subject.from_dict(scan["subject"]) - _consume_publication_reservation(repository.read_document(subject), scan_id) fingerprint = content_fingerprint(units) if scan.get("content_fingerprint") not in (None, fingerprint): raise ScreeningConflictError("A source change requires a new scan.") @@ -760,7 +758,6 @@ def finalize_publication_checkpoint(scan_id, *, repository=None): or marker.get("canonical_ref") != scan.get("units_ref") or marker.get("active_blob") != publication.get("active_blob") or publication.get("content_fingerprint") != scan.get("content_fingerprint") - or publication.get("metadata_fingerprint") != metadata_fingerprint(document) ): raise ScreeningConflictError() job_id = marker.get("job_id") or scan.get("job_id") @@ -774,7 +771,7 @@ def finalize_publication_checkpoint(scan_id, *, repository=None): published_at=scan.get("published_at") or _timestamp(), postprocess_pending=True, ) current = repository.read_document(subject) - if current.get(SCREENING_FIELD) != marker or metadata_fingerprint(current) != publication["metadata_fingerprint"]: + if current.get(SCREENING_FIELD) != marker: raise ScreeningConflictError() _log("publication_checkpoint_finalized", scan_id=scan_id, document_id=subject.document_id) return scan @@ -852,6 +849,8 @@ def heartbeat(): latest_scan = _read_scan(repository, scan_id) scan = _save_scan(repository, latest_scan, publication={ "active_blob": marker["active_blob"], + # Records the metadata released with this scan. Later metadata + # edits apply directly and do not revoke the release. "metadata_fingerprint": metadata_fingerprint({**current, **updates}), "content_fingerprint": scan["content_fingerprint"], }) @@ -1003,30 +1002,13 @@ def scan_existing_document(subject, actor_id, job_id=None, *, repository=None, s return inspect_scan(scan["id"], units, actor_id, repository=repository, storage=storage) -def queue_metadata_rescan(document, updates, actor_id, *, repository=None): - repository = _repository(repository) - if "file_name" in updates and Path(str(updates["file_name"])).suffix.lower() != Path(document["file_name"]).suffix.lower(): - raise ScreeningValidationError("Renaming a screened file cannot change its source format.") - subject = subject_from_document(document) - marker = document.get(SCREENING_FIELD) or {} +def validate_screened_metadata_update(document, updates): + """Metadata edits apply directly; a screened file keeps the source format it was inspected as.""" if ( - marker.get("state") == "pending_scan" and marker.get("origin_upload") is True - and marker.get("scan_id") and repository.get_scan(marker["scan_id"]) is None + SCREENING_FIELD in document and updates.get("file_name") is not None + and Path(str(updates["file_name"])).suffix.lower() != Path(str(document.get("file_name") or "")).suffix.lower() ): - repository.update_document(subject, updates, etag=document["_etag"]) - return None - scan = begin_scan( - subject, actor_id, repository=repository, - expected_document_etag=document["_etag"], - parent_scan_id=(document.get(SCREENING_FIELD) or {}).get("scan_id"), - ) - scan = _save_scan(repository, scan, trigger="metadata") - current = repository.read_document(subject) - repository.update_document(subject, updates, etag=current["_etag"]) - from content_screening.jobs import enqueue_document_scan - - enqueue_document_scan(subject, actor_id, scan_id=scan["id"], repository=repository) - return scan + raise ScreeningValidationError("Renaming a screened file cannot change its source format.") def reprocess_document(subject, actor_id, extraction_mode, *, repository=None, storage=None): diff --git a/application/single_app/functions_artifact_publication.py b/application/single_app/functions_artifact_publication.py index 5e0666b3b..864a83728 100644 --- a/application/single_app/functions_artifact_publication.py +++ b/application/single_app/functions_artifact_publication.py @@ -22,13 +22,12 @@ cosmos_user_documents_container, ) from content_screening.access import public_document_payload -from content_screening.contracts import SCREENING_FIELD, ScreeningError, hash_payload, subject_from_document +from content_screening.contracts import SCREENING_FIELD, ScreeningError, generated_screening_exemption from functions_appinsights import log_event from functions_artifact_publication_readiness import ( - PUBLICATION_BINDING, PUBLICATION_SCREENING_CONSUMPTION, PUBLICATION_SCREENING_RESERVATION, - SCREENING_BOOTSTRAP_REMEDY, inspect_publication_readiness, publication_binding_matches, - publication_handoff_observed, publication_processing_observation, public_publication_status, - publication_screening_reservation, + PUBLICATION_BINDING, PUBLICATION_SCREENING_HOLD_MESSAGE, inspect_publication_readiness, + publication_binding_matches, publication_handoff_observed, publication_processing_observation, + public_publication_status, ) from functions_collaboration import build_conversation_participation_context from functions_document_provenance import publication_origin_fields @@ -234,136 +233,28 @@ def _destination_document(container, receipt): return document -def _reservation_has_screening_history(document): - # Screening clients are resolved only for an enrolled publication operation. - from content_screening.repository import get_repository +def _publication_destination_admits_approval(document): + """Approval never overrides a screening hold on the destination itself. - repository = get_repository() - subject = subject_from_document(document) - if repository.get_scan(document[SCREENING_FIELD]["scan_id"]) is not None: - return True - return any( - repository.query( - kind, subject.scope_key, filters={"document_id": subject.document_id}, page_size=1, - )["items"] - for kind in ("scan", "review", "finding", "model_window", "event", "audit", "checkpoint", "work_item") - ) - - -def _enroll_screening_reservation(artifact, receipt, container): - """Capture proof only while preparing a new canonical, stored destination.""" - def enroll(current): - document = _destination_document(container, current) - proof = publication_screening_reservation(document) if document else None - if ( - proof is None or current.get("decision") - or PUBLICATION_SCREENING_CONSUMPTION in current - or "approval_queue" in (current.get("stages") or {}) - or _reservation_has_screening_history(document) - ): - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - if PUBLICATION_SCREENING_RESERVATION in current: - if current[PUBLICATION_SCREENING_RESERVATION] != proof: - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - return None - current[PUBLICATION_SCREENING_RESERVATION] = proof - return current - - return _receipt_change(artifact, receipt["id"], enroll)[0] - - -def _publication_approval_screening(receipt, document, *, settings=None, operation_id=None, queue_claimed=False): - """The bootstrap result admits only this receipt's approval-to-scan handoff.""" - if SCREENING_FIELD in document: - if (public_document_payload(document).get(SCREENING_FIELD) or {}).get("available") is True: - return "available" - elif PUBLICATION_SCREENING_RESERVATION in receipt or PUBLICATION_SCREENING_CONSUMPTION in receipt: - return None - if settings is None: - # Do not introduce settings/bootstrap initialization into readiness imports. - from functions_settings import get_settings - - settings = get_settings() + Generated destinations carry no screening marker, so approval proceeds. A destination + enrolled before generated content was exempt is approved only once its release is + available; otherwise the requester cancels and requests publication again. + """ if SCREENING_FIELD not in document: - if settings.get("enable_content_screening") is not True: - return "available" - from content_screening.service import document_requires_screening - - return None if document_requires_screening(document, settings) else "available" - if settings.get("enable_content_screening") is not True: - return None - proof = publication_screening_reservation(document) - stages = receipt.get("stages") or {} - if ( - proof is None or receipt.get(PUBLICATION_SCREENING_RESERVATION) != proof - or stages.get("create") != "complete" or stages.get("prepare") != "complete" - or "queue" in stages - or "approval_queue" in stages and not (queue_claimed and stages["approval_queue"] == "started") - ): - return None - consumption = receipt.get(PUBLICATION_SCREENING_CONSUMPTION) - decision = receipt.get("decision") - if PUBLICATION_SCREENING_CONSUMPTION in receipt: - if ( - not isinstance(consumption, dict) or not operation_id - or consumption.get("operation_id") != operation_id - or consumption.get("fingerprint") != proof["fingerprint"] - or "scan_id" in consumption - or decision and ( - decision.get("choice") != "approved" or decision.get("operation_id") != operation_id - ) - ): - return None - elif decision or queue_claimed: - return None - if _reservation_has_screening_history(document): - return None - return "bootstrap" + return True + return (public_document_payload(document).get(SCREENING_FIELD) or {}).get("available") is True -def artifact_publication_approval_available(document, *, settings=None, operation_id=None): - """Read fresh receipt and destination evidence without consuming the latch.""" +def artifact_publication_approval_available(document): + """Read fresh receipt and destination evidence before offering an approval.""" _artifact, receipt, _bound = read_artifact_publication_request(document) container = cosmos_group_documents_container if document.get("group_id") else cosmos_public_documents_container current = _destination_document(container, receipt) if current is None or current.get("_etag") != document.get("_etag"): return False - return _publication_approval_screening( - receipt, current, settings=settings, operation_id=operation_id, - ) is not None + return _publication_destination_admits_approval(current) -def consume_artifact_publication_screening_scan(document, scan_id): - """Write ahead of every scan entry; neither errors nor deletion unlatch it. - - A scan that races initial receipt preparation consumes an unverified - reservation too. Preparation cannot subsequently manufacture positive proof. - """ - artifact, receipt, _bound = read_artifact_publication_request(document) - subject = subject_from_document(document).to_dict() - - def consume(current): - proof = current.get(PUBLICATION_SCREENING_RESERVATION) - if proof is not None and (not isinstance(proof, dict) or proof.get("subject") != subject): - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - fingerprint = proof["fingerprint"] if proof else hash_payload({ - "subject": subject, "marker": document.get(SCREENING_FIELD), - }) - consumption = current.get(PUBLICATION_SCREENING_CONSUMPTION) - if PUBLICATION_SCREENING_CONSUMPTION in current: - if not isinstance(consumption, dict) or not consumption.get("operation_id"): - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - if proof and consumption.get("fingerprint") != fingerprint: - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - if "scan_id" in consumption: - return None - else: - consumption = {"operation_id": f"scan:{scan_id}", "fingerprint": fingerprint} - current[PUBLICATION_SCREENING_CONSUMPTION] = {**consumption, "scan_id": scan_id} - return current - - _receipt_change(artifact, receipt["id"], consume) - def _log_uncertain(stage, exc): log_event( @@ -627,7 +518,7 @@ def reauthorize(): if len(metadata.get(RECEIPTS_FIELD) or {}) >= MAX_ARTIFACT_PUBLICATION_REQUESTS and key not in metadata[RECEIPTS_FIELD]: raise ValueError("This artifact has reached its publication request limit.") reauthorize() - receipt, new_request = _receipt_change(artifact, key, lambda current: None if current else receipt) + receipt, _new_request = _receipt_change(artifact, key, lambda current: None if current else receipt) if receipt.get("completion_policy") != completion_policy or completion_policy is not None and receipt.get("source_receipt") != source_receipt: raise ValueError("The publication request is already bound to different inputs or a different policy.") document = _destination_document(container, receipt) @@ -635,7 +526,6 @@ def reauthorize(): return _completion_response(receipt, document) scope_args = {field: value for field, value in destination.items() if field != "workspace_scope"} scope = destination["workspace_scope"] - created_destination = False if document is None: reauthorize() if document is None and _stage(artifact, receipt, "create"): @@ -657,10 +547,12 @@ def reauthorize(): origin_fields = publication_origin_fields( user_id, artifact, source_receipt=source_receipt, destination=destination, ) + # A published artifact is generated content, so its destination is never screened. create_document( file_name=name, user_id=user_id, document_id=receipt["document_id"], num_file_chunks=0, status="Queued for processing" if scope == "personal" else "Pending approval", source_file_path=source_file_path, allow_deferred_xsd_source=scope != "personal", + screening_exemption=generated_screening_exemption(), **scope_args, **origin_fields, ) except OrchestrationFilePolicyError: @@ -668,7 +560,6 @@ def reauthorize(): except (AzureError, OSError, RuntimeError) as exc: _log_uncertain("create", exc) document = _destination_document(container, receipt) - created_destination = document is not None if document is None: return _publication_response(artifact, receipt, container) _stage(artifact, receipt, "create", complete=True) @@ -716,12 +607,6 @@ def reauthorize(): except (AzureError, OSError, RuntimeError) as exc: _log_uncertain("prepare", exc) document = _destination_document(container, receipt) - if ( - new_request and created_destination and scope == "group" and document - and document.get("generated_artifact_publication_receipt_id") == key - and SCREENING_FIELD in document - ): - receipt = _enroll_screening_reservation(artifact, receipt, container) if not document or document.get("generated_artifact_publication_receipt_id") != key: return _publication_response(artifact, receipt, container) _stage(artifact, receipt, "prepare", complete=True) @@ -1079,14 +964,13 @@ def bind(current): def decide_artifact_publication( user_id, document, choice, *, operation_guard=None, delete_destination=None, decision_link_url=None, - operation_id=None, ): """Use the destination's existing review role, while retaining a durable receipt outcome.""" require_generated_file_publication_allowed() try: return _decide_artifact_publication( user_id, document, choice, operation_guard=operation_guard, - delete_destination=delete_destination, decision_link_url=decision_link_url, operation_id=operation_id, + delete_destination=delete_destination, decision_link_url=decision_link_url, ) except OrchestrationFilePolicyError: raise @@ -1100,19 +984,17 @@ def decide_artifact_publication( def _decide_artifact_publication( user_id, document, choice, *, operation_guard=None, delete_destination=None, decision_link_url=None, - operation_id=None, ): require_generated_file_publication_allowed() with ExitStack() as resources: return _decide_artifact_publication_with_content( user_id, document, choice, resources=resources, operation_guard=operation_guard, - delete_destination=delete_destination, decision_link_url=decision_link_url, operation_id=operation_id, + delete_destination=delete_destination, decision_link_url=decision_link_url, ) def _decide_artifact_publication_with_content( user_id, document, choice, *, resources, operation_guard=None, delete_destination=None, decision_link_url=None, - operation_id=None, ): require_generated_file_publication_allowed() if choice not in {"approved", "rejected", "cancelled"}: @@ -1133,7 +1015,6 @@ def _decide_artifact_publication_with_content( receipt, _ = _receipt_change(artifact, binding["receipt_id"], lambda current: None) if not receipt or not publication_binding_matches(receipt, document): raise ValueError("The publication decision does not match its destination.") - operation_id = operation_id or (receipt.get("decision") or {}).get("operation_id") or str(uuid.uuid4()) destination = receipt["destination"] scope = destination["workspace_scope"] if scope not in {"group", "public"}: @@ -1189,24 +1070,13 @@ def record_decision(current): previous = current.get("decision") if previous and previous.get("choice") != choice: raise ValueError("A different publication decision already committed.") - bootstrap = False - if choice == "approved": - admission = _publication_approval_screening(current, current_document, operation_id=operation_id) - if admission is None: - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) - bootstrap = admission == "bootstrap" - if bootstrap and PUBLICATION_SCREENING_CONSUMPTION not in current: - current[PUBLICATION_SCREENING_CONSUMPTION] = { - "operation_id": operation_id, - "fingerprint": current[PUBLICATION_SCREENING_RESERVATION]["fingerprint"], - } + if choice == "approved" and not _publication_destination_admits_approval(current_document): + raise ValueError(PUBLICATION_SCREENING_HOLD_MESSAGE) if previous: return None current["decision"] = { "choice": choice, "actor_user_id": user_id, "decided_at": datetime.now(timezone.utc).isoformat(), } - if bootstrap: - current["decision"]["operation_id"] = operation_id return current receipt, _ = _receipt_change(artifact, receipt["id"], record_decision) @@ -1227,10 +1097,8 @@ def record_decision(current): _authorize_destination(receipt["actor_user_id"], destination) receipt, _ = _receipt_change(artifact, receipt["id"], lambda saved: None) current = authorize_decision() - if _publication_approval_screening( - receipt, current, operation_id=operation_id, queue_claimed=True, - ) is None: - raise ValueError(SCREENING_BOOTSTRAP_REMEDY) + if not _publication_destination_admits_approval(current): + raise ValueError(PUBLICATION_SCREENING_HOLD_MESSAGE) queue_generated_document_processing( document_id=receipt["document_id"], owner_user_id=receipt["actor_user_id"], normalized_file_name=receipt["file_name"], file_content_bytes=source_bytes, **scope_args, diff --git a/application/single_app/functions_artifact_publication_readiness.py b/application/single_app/functions_artifact_publication_readiness.py index ae53b498f..a3c57af56 100644 --- a/application/single_app/functions_artifact_publication_readiness.py +++ b/application/single_app/functions_artifact_publication_readiness.py @@ -9,21 +9,17 @@ from azure.core import MatchConditions from azure.cosmos.exceptions import CosmosHttpResponseError -from content_screening.access import PROVENANCE_FIELD, assert_document_available +from content_screening.access import assert_document_available from content_screening.contracts import ( AVAILABLE_STATES, HELD_STATES, SCREENING_FIELD, DocumentHeldError, - hash_payload, subject_from_document, ) PUBLICATION_BINDING = "generated_artifact_publication_binding" PUBLICATION_PROCESSING = "generated_artifact_publication_processing" -PUBLICATION_SCREENING_RESERVATION = "screening_reservation" -PUBLICATION_SCREENING_CONSUMPTION = "screening_reservation_consumption" -SCREENING_BOOTSTRAP_REMEDY = ( - "This screening reservation cannot start publication. Cancel the pending publication request " - "and request publication again. If approval already committed, reconcile its existing handoff; " - "do not queue another copy." +PUBLICATION_SCREENING_HOLD_MESSAGE = ( + "This publication destination is held by content screening and cannot be approved. " + "Cancel the pending publication request and request publication again." ) PUBLICATION_STATUS_FIELDS = ( "version", "id", "document_id", "document_version", "destination", "completion_policy", @@ -32,58 +28,6 @@ ) -def publication_screening_reservation(document): - """Identify only a pristine origin reservation, never a scan or a clearance. - - The reserved UUID and original marker timestamp distinguish successive - reservations for the same subject. Cosmos resource identity, when present, - also binds the proof to the stored document instance rather than its name. - """ - marker = document.get(SCREENING_FIELD) - if not isinstance(marker, dict) or set(marker) != { - "schema_version", "state", "source_revision", "scan_id", "content_fingerprint", - "finding_count", "review_required", "generation", "origin_upload", "updated_at", - }: - return None - if ( - not document.get("group_id") or document.get("public_workspace_id") - or type(document.get("version")) is not int or document["version"] != 1 - or document.get("revision_family_id") not in (None, document.get("id")) - or not _current_revision(document) - or type(marker.get("schema_version")) is not int or marker["schema_version"] != 1 - or marker.get("state") != "pending_scan" or marker.get("source_revision") != "1" - or type(marker.get("generation")) is not int or marker["generation"] != 1 - or marker.get("origin_upload") is not True or marker.get("review_required") is not False - or type(marker.get("finding_count")) is not int or marker["finding_count"] != 0 - or marker.get("content_fingerprint") is not None - or not isinstance(marker.get("scan_id"), str) or not marker["scan_id"] - or not isinstance(marker.get("updated_at"), str) or not marker["updated_at"] - or PUBLICATION_PROCESSING in document or PROVENANCE_FIELD in document - ): - return None - if any(document.get(field) for field in ( - "blob_path", "archived_blob_path", "original_blob_path", "blob_etag", "blob_content_hash", - "source_ref", "canonical_ref", "units_ref", "result_ref", "active_manifest_id", - "active_content_manifest", "source_file_available", "enhanced_citations", - "content", "text", "chunks", "pages", "title", "abstract", "keywords", "authors", - "publication_date", "vision_analysis", - )): - return None - if any( - field in document and (type(document[field]) is not int or document[field] != 0) - for field in ("num_chunks", "num_file_chunks", "number_of_pages", "current_file_chunk") - ) or document.get("percentage_complete", 0) != 0: - return None - subject = subject_from_document(document).to_dict() - return { - "schema_version": 1, "subject": subject, - "fingerprint": hash_payload({ - "subject": subject, "marker": marker, - "resource_id": document.get("_rid"), "upload_date": document.get("upload_date"), - }), - } - - def public_publication_status(value): if not isinstance(value, dict) or value.get("version") != 1: raise ValueError("The publication completion status is unavailable.") @@ -267,6 +211,8 @@ def _index_count(receipt, document): def inspect_publication_readiness(receipt, document, *, available_reader=None, index_count=None, check_index=True): """Read native proof; never queue, approve, resume, index, or mutate a document.""" processing = publication_processing_observation(receipt, document) + # Generated destinations carry no screening marker, so screening is already complete + # for them; the destination access check below still applies. observation = {"processing": processing, "screening": "not_required", "index": "pending"} if not publication_binding_matches(receipt, document): return {**observation, "reason_code": "publication_revision_changed"} diff --git a/application/single_app/functions_documents.py b/application/single_app/functions_documents.py index 009623ea8..06ea28076 100644 --- a/application/single_app/functions_documents.py +++ b/application/single_app/functions_documents.py @@ -14,12 +14,14 @@ from azure.core import MatchConditions from azure.core.exceptions import AzureError, ResourceExistsError from content_screening.contracts import ( + SCREENING_EXEMPTION_FIELD, SCREENING_FIELD, DocumentHeldError, ScreeningConflictError, ScreeningError, ScreeningValidationError, document_is_available, + is_generated_screening_exempt, require_document_available, subject_from_document, ) @@ -30,7 +32,6 @@ read_available_document_bytes, ) from content_screening.extraction import ( - CONTENT_METADATA_FIELDS, current_extraction, is_publication, ) @@ -40,8 +41,8 @@ prepare_document_deletion, prepare_document_upload, process_screened_upload, - queue_metadata_rescan, reprocess_document, + validate_screened_metadata_update, ) from config import * from functions_appinsights import log_event @@ -2047,16 +2048,22 @@ def create_document( allow_deferred_xsd_source=False, origin=None, server_tags=None, + screening_exemption=None, ): """Create or version a document record. - ``origin`` and ``server_tags`` are server-only: callers pass them only from server-held - workflow or chat bindings, never from client input. Each version stores only its own origin. + ``origin``, ``server_tags`` and ``screening_exemption`` are server-only: callers pass them only + from server-held workflow, chat, or generated-content bindings, never from client input. Each + version stores only its own origin and exemption. """ if origin is not None: # Validated before any earlier revision is archived, so a malformed origin can never # leave the document family without a current version. origin = validate_origin(origin) + if screening_exemption is not None and not is_generated_screening_exempt( + {SCREENING_EXEMPTION_FIELD: screening_exemption} + ): + raise ScreeningValidationError("The screening exemption is invalid.") current_time = datetime.now(timezone.utc).strftime('%Y-%m-%dT%H:%M:%SZ') is_group = group_id is not None is_public_workspace = public_workspace_id is not None @@ -2364,6 +2371,9 @@ def create_document( "xsd_blob_etag": staged_xsd_source.get("blob_etag"), }) apply_document_provenance(document_metadata, origin=origin, server_tags=server_tags) + if screening_exemption is not None: + # Stored before the marker is chosen, so a generated version is never enrolled. + document_metadata[SCREENING_EXEMPTION_FIELD] = dict(screening_exemption) screening_marker = initial_document_marker(document_metadata) if screening_marker is not None: document_metadata[SCREENING_FIELD] = screening_marker @@ -3532,7 +3542,9 @@ def calculate_processing_percentage(doc_metadata): return max(final_pct, current_pct) def update_document(**kwargs): - if SCREENING_FIELD in kwargs: + # Screening state and every screening_* field, including the generated-content + # exemption, are server-managed and set only when a version is created. + if SCREENING_FIELD in kwargs or any(str(name).startswith("screening_") for name in kwargs): raise ScreeningValidationError("Screening state is managed by the review workflow.") if any(field_name in kwargs for field_name in ORIGIN_FIELD_NAMES): # Origin is written only when a version is created; no update path may set or change it. @@ -3652,29 +3664,13 @@ def update_document(**kwargs): kwargs["status"] = "Content review required" if marker.get("review_required") else "Content screening pending" if kwargs.get("percentage_complete") == 100: kwargs["percentage_complete"] = 99 - content_changed = any( - key in kwargs and kwargs[key] is not None and kwargs[key] != existing_document.get(key) - for key in CONTENT_METADATA_FIELDS - ) if ( - marker is not None and content_changed and capture is None + marker is not None and capture is None and not is_publication(subject_from_document(existing_document), marker.get("scan_id")) ): - metadata_updates = { - key: value for key, value in kwargs.items() - if key not in {"document_id", "user_id", "group_id", "public_workspace_id"} - and value is not None - } - if operation_guard is not None: - operation_guard() - queue_metadata_rescan(existing_document, metadata_updates, user_id) - if strict: - saved_document = cosmos_container.read_item(item=document_id, partition_key=document_id) - projection = sync_document_access_index_for_document_fail_open(saved_document, operation="document_updated") - if not projection.get("success"): - raise DocumentMutationPropagationError("Document saved, but its access projection could not be updated.") - return saved_document - return + # Metadata edits, including model-generated metadata, apply directly and never + # start a new hold. Only the inspected source format of the file stays fixed. + validate_screened_metadata_update(existing_document, kwargs) original_percentage = existing_document.get('percentage_complete', 0) # Store for comparison # 2. Apply updates from kwargs @@ -3753,7 +3749,11 @@ def update_document(**kwargs): # This happens regardless of 'update_occurred' flag because the *intent* from kwargs might trigger it, # even if the main doc update didn't happen (e.g., only percentage changed). # However, it's better to only do this if the relevant fields *actually* changed. - if (update_occurred or strict) and updated_fields_requiring_chunk_sync and marker is None: + # A held screened document has no released chunks; publication rebuilds them. + if ( + (update_occurred or strict) and updated_fields_requiring_chunk_sync + and (marker is None or document_is_available(existing_document)) + ): try: chunks_to_update = get_all_chunks( document_id, @@ -13249,6 +13249,10 @@ def propagate_tags_to_blob_metadata( or (public_workspace_id is not None and doc_item.get("public_workspace_id") != public_workspace_id) ): raise ScreeningConflictError() + if SCREENING_FIELD in doc_item: + # A screened release pins its blob by ETag and content hash. Rewriting the blob's + # metadata would make the released file unreadable, so tags stay in Cosmos and Search. + return if strict and not _has_persisted_blob_reference(doc_item) and not doc_item.get("enhanced_citations"): return storage_account_container_name, blob_path = get_document_blob_storage_info( diff --git a/application/single_app/functions_group_document_management.py b/application/single_app/functions_group_document_management.py index 4b2b1b7a3..df6e26507 100644 --- a/application/single_app/functions_group_document_management.py +++ b/application/single_app/functions_group_document_management.py @@ -285,11 +285,11 @@ def update_group_document_metadata( updated_fields={name: "[updated]" for name in changes} if SCREENING_FIELD in document else changes, file_type=document.get("file_type"), ) - queued = SCREENING_FIELD in document and any(document.get(name) != value for name, value in changes.items()) + # Metadata edits apply directly; they never start a new screening hold. return { - "message": "Metadata saved and queued for content screening." if queued else "Group document metadata updated.", + "message": "Group document metadata updated.", "document_id": document_id, "group_id": group_id, - "updated_fields": list(changes), "status": "queued" if queued else "updated", + "updated_fields": list(changes), "status": "updated", } diff --git a/application/single_app/functions_group_document_publication.py b/application/single_app/functions_group_document_publication.py index bcd1b3848..e533ff562 100644 --- a/application/single_app/functions_group_document_publication.py +++ b/application/single_app/functions_group_document_publication.py @@ -11,8 +11,7 @@ from functions_appinsights import log_event import functions_artifact_publication as publication from functions_artifact_publication_readiness import ( - PUBLICATION_BINDING, PUBLICATION_SCREENING_CONSUMPTION, SCREENING_BOOTSTRAP_REMEDY, - publication_handoff_observed, + PUBLICATION_BINDING, PUBLICATION_SCREENING_HOLD_MESSAGE, publication_handoff_observed, ) from functions_documents import delete_document_revision, select_current_documents from functions_group_document_access import ( @@ -53,31 +52,9 @@ def _has_publication(document): ) -def _bootstrap_retry_operation_id(document, receipt, user_id): - operation = document.get(GROUP_DOCUMENT_COLLABORATION_OPERATION) or {} - consumption = receipt.get(PUBLICATION_SCREENING_CONSUMPTION) or {} - if ( - isinstance(operation, dict) and isinstance(consumption, dict) - and operation.get("schema_version") == 1 and operation.get("action") == "approve_artifact" - and operation.get("phase") in {"executing", "repair"} - and operation.get("actor_user_id") == user_id - and operation.get("actor_group_id") == operation.get("source_group_id") == document.get("group_id") - and operation.get("document_id") == document.get("id") - and operation.get("document_version") == document.get("version") - and isinstance(operation.get("id"), str) and operation["id"] - and isinstance(operation.get("execution_token"), str) and operation["execution_token"] - and consumption.get("operation_id") == operation["id"] and "scan_id" not in consumption - and "approval_queue" not in (receipt.get("stages") or {}) - ): - return operation["id"] - return None - - -def _approval_available(document, settings, *, operation_id=None): +def _approval_available(document): try: - return publication.artifact_publication_approval_available( - document, settings=settings, operation_id=operation_id, - ) + return publication.artifact_publication_approval_available(document) except ScreeningError as error: log_event( "[DOCUMENTS] Publication screening admission could not be established.", @@ -90,7 +67,7 @@ def _approval_available(document, settings, *, operation_id=None): def group_publication_view(document, user_id, group_id, *, context): if not _has_publication(document): return None - _group, role, settings, supported = context + _group, role, _settings, supported = context status = "pending_approval" if group_document_approval_pending(document) else document.get("generated_artifact_promotion_status") if status not in {"pending_approval", "approved", "approval_failed", "rejected", "cancelled"}: status = "unavailable" @@ -99,9 +76,8 @@ def group_publication_view(document, user_id, group_id, *, context): try: _artifact, receipt, _bound = publication.read_artifact_publication_request(document) requester = receipt["actor_user_id"] - retry_id = _bootstrap_retry_operation_id(document, receipt, user_id) operation = document.get(GROUP_DOCUMENT_COLLABORATION_OPERATION) or {} - can_approve = operation.get("phase") != "executing" or retry_id is not None + can_approve = operation.get("phase") != "executing" decision = (receipt.get("decision") or {}).get("choice") if decision == "approved": status = "approved" if (receipt.get("stages") or {}).get("approval_queue") == "complete" else "approval_failed" @@ -115,7 +91,7 @@ def group_publication_view(document, user_id, group_id, *, context): actions.append("reject_artifact") if ( can_approve and is_current_group_document(document) - and _approval_available(document, settings, operation_id=retry_id) + and _approval_available(document) ): actions.append("approve_artifact") if requester == user_id: @@ -126,7 +102,7 @@ def group_publication_view(document, user_id, group_id, *, context): if ( can_approve and role in GROUP_DOCUMENT_MANAGER_ROLES and is_current_group_document(document) - and _approval_available(document, settings, operation_id=retry_id) + and _approval_available(document) ): actions.append("approve_artifact") elif decision == "rejected" and role in GROUP_DOCUMENT_MANAGER_ROLES: @@ -173,7 +149,7 @@ def decide_group_document_publication(user_id, group_id, document_id, action, pa require_payload(payload, {"expected_etag"}, {"expected_etag"}) validate_group_document_id(document_id) - group, role, settings, supported = _context(user_id, group_id) + group, role, _settings, supported = _context(user_id, group_id) if action not in PUBLICATION_ACTIONS or action not in supported: raise GroupDocumentCollaborationError("publication_forbidden", "Your current group role or status cannot make this decision.", 403) document = read_group_document_record(document_id) @@ -196,18 +172,17 @@ def decide_group_document_publication(user_id, group_id, document_id, action, pa raise GroupDocumentCollaborationError("manager_required", "A destination document manager must make this decision.", 403) if choice == "approved" and not is_current_group_document(document): raise GroupDocumentCollaborationError("revision_unavailable", "A historical publication request cannot be approved.", 409) - retry_id = _bootstrap_retry_operation_id(document, receipt, user_id) if choice == "approved" else None - if choice == "approved" and not _approval_available(document, settings, operation_id=retry_id): - raise GroupDocumentCollaborationError("publication_unavailable", SCREENING_BOOTSTRAP_REMEDY, 409) + if choice == "approved" and not _approval_available(document): + raise GroupDocumentCollaborationError("publication_unavailable", PUBLICATION_SCREENING_HOLD_MESSAGE, 409) previous_operation = _operation(document) if ( previous_operation and _unfinished(previous_operation) - and previous_operation.get("phase") in {"executing", "uncertain"} and not retry_id + and previous_operation.get("phase") in {"executing", "uncertain"} ): raise GroupDocumentCollaborationError("operation_busy", "Reconcile the existing document operation before making another decision.", 409) if previous_operation and previous_operation.get("phase") not in {"complete", None} and previous_operation.get("action") != action: raise GroupDocumentCollaborationError("operation_busy", "A different collaboration operation needs reconciliation.", 409) - operation_id = retry_id or str(uuid.uuid4()) + operation_id = str(uuid.uuid4()) execution_token = str(uuid.uuid4()) operation = { "schema_version": 1, "id": operation_id, "document_id": document_id, @@ -297,7 +272,6 @@ def cleanup_guard(document_id=None): publication.decide_artifact_publication( user_id, document, choice, operation_guard=guard, delete_destination=cleanup, decision_link_url=f"/v2/groups/{quote(group_id, safe='')}/documents?document_id={quote(document_id, safe='')}", - operation_id=operation_id, ) except Exception as error: log_event( diff --git a/application/single_app/functions_public_document_management.py b/application/single_app/functions_public_document_management.py index d953ab76a..e410c9940 100644 --- a/application/single_app/functions_public_document_management.py +++ b/application/single_app/functions_public_document_management.py @@ -291,11 +291,11 @@ def update_public_document_metadata( updated_fields={name: "[updated]" for name in changes} if SCREENING_FIELD in document else changes, file_type=document.get("file_type"), ) - queued = SCREENING_FIELD in document and any(document.get(name) != value for name, value in changes.items()) + # Metadata edits apply directly; they never start a new screening hold. return { - "message": "Metadata saved and queued for content screening." if queued else "Public document metadata updated.", + "message": "Public document metadata updated.", "document_id": document_id, "public_workspace_id": workspace_id, - "updated_fields": list(changes), "status": "queued" if queued else "updated", + "updated_fields": list(changes), "status": "updated", } diff --git a/application/single_app/functions_public_document_publication.py b/application/single_app/functions_public_document_publication.py index f828a43e6..74a9eb79d 100644 --- a/application/single_app/functions_public_document_publication.py +++ b/application/single_app/functions_public_document_publication.py @@ -2,11 +2,10 @@ """Scoped adapters for the existing durable artifact-publication lifecycle, targeting an explicit public workspace. -Public workspaces reuse the same shared artifact-publication engine, screening -bootstrap and consume-latch as groups; this module only substitutes the public -workspace scope for the group scope. There is no public share relationship yet -(that arrives with M3D), so this file carries only the generated-artifact -decision surface. +Public workspaces reuse the same shared artifact-publication engine as groups; +this module only substitutes the public workspace scope for the group scope. +There is no public share relationship yet (that arrives with M3D), so this file +carries only the generated-artifact decision surface. """ from urllib.parse import quote @@ -17,8 +16,7 @@ from functions_appinsights import log_event import functions_artifact_publication as publication from functions_artifact_publication_readiness import ( - PUBLICATION_SCREENING_CONSUMPTION, SCREENING_BOOTSTRAP_REMEDY, - publication_handoff_observed, + PUBLICATION_SCREENING_HOLD_MESSAGE, publication_handoff_observed, ) from functions_documents import delete_document_revision, select_current_documents from functions_public_document_access import ( @@ -61,31 +59,9 @@ def _has_publication(document): return public_document_has_publication(document) -def _bootstrap_retry_operation_id(document, receipt, user_id): - operation = document.get(PUBLIC_DOCUMENT_COLLABORATION_OPERATION) or {} - consumption = receipt.get(PUBLICATION_SCREENING_CONSUMPTION) or {} - if ( - isinstance(operation, dict) and isinstance(consumption, dict) - and operation.get("schema_version") == 1 and operation.get("action") == "approve_artifact" - and operation.get("phase") in {"executing", "repair"} - and operation.get("actor_user_id") == user_id - and operation.get("actor_public_workspace_id") == operation.get("source_public_workspace_id") == document.get("public_workspace_id") - and operation.get("document_id") == document.get("id") - and operation.get("document_version") == document.get("version") - and isinstance(operation.get("id"), str) and operation["id"] - and isinstance(operation.get("execution_token"), str) and operation["execution_token"] - and consumption.get("operation_id") == operation["id"] and "scan_id" not in consumption - and "approval_queue" not in (receipt.get("stages") or {}) - ): - return operation["id"] - return None - - -def _approval_available(document, settings, *, operation_id=None): +def _approval_available(document): try: - return publication.artifact_publication_approval_available( - document, settings=settings, operation_id=operation_id, - ) + return publication.artifact_publication_approval_available(document) except ScreeningError as error: log_event( "[DOCUMENTS] Publication screening admission could not be established.", @@ -98,7 +74,7 @@ def _approval_available(document, settings, *, operation_id=None): def public_publication_view(document, user_id, public_workspace_id, *, context): if not _has_publication(document): return None - _workspace, role, settings, supported = context + _workspace, role, _settings, supported = context status = "pending_approval" if public_document_approval_pending(document) else document.get("generated_artifact_promotion_status") if status not in {"pending_approval", "approved", "approval_failed", "rejected", "cancelled"}: status = "unavailable" @@ -107,9 +83,8 @@ def public_publication_view(document, user_id, public_workspace_id, *, context): try: _artifact, receipt, _bound = publication.read_artifact_publication_request(document) requester = receipt["actor_user_id"] - retry_id = _bootstrap_retry_operation_id(document, receipt, user_id) operation = document.get(PUBLIC_DOCUMENT_COLLABORATION_OPERATION) or {} - can_approve = operation.get("phase") != "executing" or retry_id is not None + can_approve = operation.get("phase") != "executing" decision = (receipt.get("decision") or {}).get("choice") if decision == "approved": status = "approved" if (receipt.get("stages") or {}).get("approval_queue") == "complete" else "approval_failed" @@ -123,7 +98,7 @@ def public_publication_view(document, user_id, public_workspace_id, *, context): actions.append("reject_artifact") if ( can_approve and is_current_public_document(document) - and _approval_available(document, settings, operation_id=retry_id) + and _approval_available(document) ): actions.append("approve_artifact") if requester == user_id: @@ -134,7 +109,7 @@ def public_publication_view(document, user_id, public_workspace_id, *, context): if ( can_approve and role in PUBLIC_DOCUMENT_MANAGER_ROLES and is_current_public_document(document) - and _approval_available(document, settings, operation_id=retry_id) + and _approval_available(document) ): actions.append("approve_artifact") elif decision == "rejected" and role in PUBLIC_DOCUMENT_MANAGER_ROLES: @@ -181,7 +156,7 @@ def decide_public_document_publication(user_id, public_workspace_id, document_id require_payload(payload, {"expected_etag"}, {"expected_etag"}) validate_public_document_id(document_id) - workspace, role, settings, supported = _context(user_id, public_workspace_id) + workspace, role, _settings, supported = _context(user_id, public_workspace_id) if action not in PUBLICATION_ACTIONS or action not in supported: raise PublicDocumentCollaborationError("publication_forbidden", "Your current workspace role or status cannot make this decision.", 403) document = read_public_document_record(document_id) @@ -207,18 +182,17 @@ def decide_public_document_publication(user_id, public_workspace_id, document_id raise PublicDocumentCollaborationError("manager_required", "A hosting workspace document manager must make this decision.", 403) if choice == "approved" and not is_current_public_document(document): raise PublicDocumentCollaborationError("revision_unavailable", "A historical publication request cannot be approved.", 409) - retry_id = _bootstrap_retry_operation_id(document, receipt, user_id) if choice == "approved" else None - if choice == "approved" and not _approval_available(document, settings, operation_id=retry_id): - raise PublicDocumentCollaborationError("publication_unavailable", SCREENING_BOOTSTRAP_REMEDY, 409) + if choice == "approved" and not _approval_available(document): + raise PublicDocumentCollaborationError("publication_unavailable", PUBLICATION_SCREENING_HOLD_MESSAGE, 409) previous_operation = _operation(document) if ( previous_operation and _unfinished(previous_operation) - and previous_operation.get("phase") in {"executing", "uncertain"} and not retry_id + and previous_operation.get("phase") in {"executing", "uncertain"} ): raise PublicDocumentCollaborationError("operation_busy", "Reconcile the existing document operation before making another decision.", 409) if previous_operation and previous_operation.get("phase") not in {"complete", None} and previous_operation.get("action") != action: raise PublicDocumentCollaborationError("operation_busy", "A different collaboration operation needs reconciliation.", 409) - operation_id = retry_id or str(uuid.uuid4()) + operation_id = str(uuid.uuid4()) execution_token = str(uuid.uuid4()) operation = { "schema_version": 1, "id": operation_id, "document_id": document_id, @@ -308,7 +282,6 @@ def cleanup_guard(document_id=None): publication.decide_artifact_publication( user_id, document, choice, operation_guard=guard, delete_destination=cleanup, decision_link_url=f"/v2/public/{quote(public_workspace_id, safe='')}/documents?document_id={quote(document_id, safe='')}", - operation_id=operation_id, ) except Exception as error: log_event( diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index b4beb5260..3295f4203 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -43,6 +43,7 @@ log_workflow_creation, ) from functions_appinsights import log_event +from content_screening.contracts import generated_screening_exemption from content_screening.service import prepare_document_upload from functions_citation_tracking import rebuild_conversation_used_documents from functions_authentication import ( @@ -2866,6 +2867,7 @@ def _upload_generated_document_for_current_user( num_file_chunks=0, status=initial_status, source_file_path=temp_file_path, + screening_exemption=generated_screening_exemption(), ) update_document( document_id=document_id, @@ -2881,6 +2883,7 @@ def _upload_generated_document_for_current_user( num_file_chunks=0, status=initial_status, source_file_path=temp_file_path, + screening_exemption=generated_screening_exemption(), ) update_document( document_id=document_id, diff --git a/docs/explanation/features/CONTENT_SCREENING_FRAMEWORK.md b/docs/explanation/features/CONTENT_SCREENING_FRAMEWORK.md index 17accc56e..8d3db479b 100644 --- a/docs/explanation/features/CONTENT_SCREENING_FRAMEWORK.md +++ b/docs/explanation/features/CONTENT_SCREENING_FRAMEWORK.md @@ -6,7 +6,7 @@ Content screening creates an admission checkpoint between document extraction an **Implemented in version: 0.261.106.** The application version is managed in `application\single_app\config.py`. -**Current documentation version: 0.261.139.** Gather / Reason / Render orchestration integration with chat checkpoints was implemented in 0.261.131 and remains the current orchestration path as of 0.261.139. Chat checkpoints and strict retained-result source-authority errors were implemented in 0.261.127; enabled-empty policy configuration was implemented in 0.261.114; classic/V2 policy-editor alignment was implemented in 0.261.108; the original framework implementation remains 0.261.106. +**Current documentation version: 0.261.230.** Upload-only intake was implemented in 0.261.230: documents SimpleChat generates are never screened, and metadata edits, including model-generated metadata, apply without a new hold. Gather / Reason / Render orchestration integration with chat checkpoints was implemented in 0.261.131. Chat checkpoints and strict retained-result source-authority errors were implemented in 0.261.127; enabled-empty policy configuration was implemented in 0.261.114; classic/V2 policy-editor alignment was implemented in 0.261.108; the original framework implementation remains 0.261.106. **Dependencies:** Enhanced Citations and its configured storage account, the existing Cosmos DB and workspace knowledge services, and an approved model connection when a policy includes model evaluation. @@ -18,13 +18,19 @@ The motivating case is text that is easy to miss in a visual document, such as w The reusable `application\single_app\content_screening\` package separates policy/evaluation from the workspace-document adapter. Its contracts describe a subject, source revision, canonical content units, effective policy, grounded findings, and coverage. +### What gets screened + +Since **0.261.230**, screening checks only documents that enter a workspace from outside SimpleChat: a user upload to a personal, group or public workspace (including a file attached in chat and saved to a workspace) or File Sync. Any other way a document reaches a workspace stays screened unless it is explicitly marked as generated. + +Documents that SimpleChat generates are never screened, neither when they are created nor later. That covers documents the SimpleChat agent action saves to a workspace, such as Markdown, Word and PowerPoint files, and chat or workflow artifacts published into a workspace. The code that creates them stores a server-managed `screening_exemption` field (`{"reason": "generated", "version": 1}`) on the new document version before the initial screening marker is chosen, so the version gets no marker. The exemption belongs to that one version: a later user upload of the same file is screened. Clients cannot set or change the field. The document API guards reject any request key that starts with `screening`, `update_document` refuses every `screening_*` field, and document responses never include it. A document that already carries a screening marker stays screened even if it also has the exemption. Scan jobs skip generated documents and record them as skipped with `screening_not_required`. + New enrolled documents are held before ordinary processing is queued. Their source and extracted content are retained privately. The existing processors supply canonical pages, text, table cells, and transcript segments without embedding or publishing early chunks. Publication happens only after the complete verdict and any required human decision. The document record contains a small screening marker. Detailed evidence and canonical content live in private artifacts in the Enhanced Citations storage account; policy, scan, job, and audit metadata live in the screening repository. Normal application settings do not contain the detailed finding payload. Cosmos state is authoritative. Search projections, cached results, native table reads, source previews, historical citations, and repeated source-context use must honor the current document decision. A stale index entry or cached snippet is not permission to use the document. -Release also records the exact approved Blob and content-derived metadata fingerprints. Restoring a document marker without its completed scan, or changing an abstract or tag after inspection, does not create a valid clearance. Ordinary lists return status-only metadata when that proof is unavailable. +Release also records the exact approved Blob and content fingerprints. Restoring a document marker without its completed scan does not create a valid clearance. Since **0.261.230**, editing a released document's title, abstract, tags or other metadata keeps its clearance: the edit applies directly, is not screened, and does not start a new hold. A rename still cannot change a screened file's extension, and tag edits never rewrite the released Blob's metadata, because the release is bound to that Blob's ETag and content hash. Ordinary lists return status-only metadata when release proof is unavailable. ## Policies and evaluators @@ -104,7 +110,9 @@ Cancellation does not release a document already being inspected. Concurrent sou Source-bound conversation history and exports withhold unavailable attachments and tool evidence. Remediated workspace attachments are refreshed from the current clean representation. Legacy native tool results that have no matching revision proof must be regenerated rather than borrowing a later approval. -Automatic metadata generation uses admitted source content. New metadata changes are held and inspected before they replace the knowledge projection. +Automatic metadata generation uses admitted source content. Since **0.261.230**, the metadata it generates is applied to the released document directly, the same way as a user's metadata edit, without a new hold or scan. + +Publishing a generated artifact to a group or public workspace no longer reserves a screening scan for its destination. The destination is generated content, so it has no screening marker and its approval hands it straight to ordinary processing. A destination that was already held by screening before 0.261.230 cannot be approved; cancel that request and request publication again. ## Configuration and usage @@ -136,6 +144,8 @@ Enabled-empty policy coverage also exercises first activation, save/reload witho The core cases include a last-page finding, complete window coverage, regex deadlines, strict model responses, sticky review holds, authorization, revision conflicts, safe derivatives, and recovery from partial publication. +`functional_tests\test_content_screening_upload_only_intake.py` covers upload-only intake: generated and published documents get no marker while user uploads, chat uploads and File Sync do; clients cannot set the exemption through any guarded document route; scan jobs skip generated documents; metadata edits keep a release; and model-generated metadata applies without a hold. + `functional_tests\test_orchestration_source_access.py` exercises strict metadata and scope-service failures, the real mixed-source/search resolver, retained-result discovery and reopen, unchanged legacy defaults, caught-error model fences, concurrent scope isolation and network-blocked cold imports in normal and optimized Python. `test_content_screening_access.py` continues to cover the original default-mode sanitization and model-fallback fence. The document adapter covers workspace knowledge, including chat files handed off to a workspace. **0.261.127** adds [chat text checkpoints]({{ '/explanation/features/CHAT_CONTENT_CHECKS/' | relative_url }}) that reuse its global baseline without changing document holds or workspace review. Since **0.261.131**, Gather / Reason / Render orchestration replies use the same output checkpoint before publication, including model-free delivery. Chat-only attachment screening, outbound web-search preflight, and agent-to-agent message inspection remain outside these checkpoints. diff --git a/docs/explanation/features/DOCUMENT_PROVENANCE.md b/docs/explanation/features/DOCUMENT_PROVENANCE.md index 40ff5c206..669351fc6 100644 --- a/docs/explanation/features/DOCUMENT_PROVENANCE.md +++ b/docs/explanation/features/DOCUMENT_PROVENANCE.md @@ -152,10 +152,15 @@ values aren't valid, the document is saved without one and a warning is logged with `log_event`. Publication stamps the origin when it creates the destination document. -Approval, content screening, and `queue_generated_document_processing` all work -on that same document and never re-create it, so the origin survives them. The -create stage runs once per publication receipt, so a replayed publication -doesn't restamp the origin or add the tag twice. +Approval and `queue_generated_document_processing` work on that same document +and never re-create it, so the origin survives them. Since 0.261.230 the +destination also gets the server-managed `screening_exemption` field, so content +screening doesn't run on it. The create stage runs once per publication receipt, +so a replayed publication doesn't restamp the origin or add the tag twice. + +The `origin` field is not what exempts a document from screening. A `chat` +origin also marks a file a user attached in chat and saved to a workspace, +which is an upload and is still screened. The chat upload route builds its origin only from the conversation it has already authorized and the file message ID it generates. It never reads origin diff --git a/docs/explanation/features/GROUP_DOCUMENT_COLLABORATION_APIS.md b/docs/explanation/features/GROUP_DOCUMENT_COLLABORATION_APIS.md index ef2796034..3d61a1511 100644 --- a/docs/explanation/features/GROUP_DOCUMENT_COLLABORATION_APIS.md +++ b/docs/explanation/features/GROUP_DOCUMENT_COLLABORATION_APIS.md @@ -193,63 +193,24 @@ Collaboration changes coordinate with document projection writes through from republishing a revoked access list. See [Group Document Projection Coordination](GROUP_DOCUMENT_PROJECTION_COORDINATION.md). -## Screening bootstrap for artifact approval +## Screening and artifact approval -Approving a generated artifact is the operation whose whole purpose is to -*start* screening on the destination. That creates a bootstrap problem: the -destination shell already carries a reserved screening marker, and a scoped -adapter that denies every non-available marker would deny the approval that -would have started the scan, deadlocking every new artifact approval once -screening is enabled. +Since version 0.261.230, a published artifact is generated content. Its +destination document is created with the server-managed `screening_exemption` +field, so it never gets a screening marker, and approving it hands it straight +to ordinary processing. The publication receipt no longer reserves or consumes a +screening scan, and approval needs no scan-start proof. -Two facts make the naive fixes unsafe: +Approval still never overrides a screening hold on the destination itself. A +destination that carries a screening marker can be approved only when its +release is available. In practice that is a destination created before +0.261.230 with screening on. If it is still held, the approval is refused with +409 `publication_unavailable`, and the remedy is to cancel the request and +request publication again, which creates a new destination that isn't screened. -- **A reserved `scan_id` always exists.** The initial document marker carries - one before anything runs, so its presence is reservation *identity* and is - never evidence that a scan ran, nor a deny signal on its own. -- **Absence of a scan row proves nothing.** It cannot distinguish "never - started" from "started, errored, and the row was later removed or reset", - which would otherwise be a laundering path. - -Positive proof is therefore recorded, not inferred. The existing publication -receipt owns two fields: - -| Field | Meaning | -|---|---| -| `screening_reservation` | The exact stored subject plus a fingerprint derived from the original reserved identifier, the full initial marker, the upload timestamp, and the Cosmos resource identity when present | -| `screening_reservation_consumption` | A monotonic latch recording the consuming `operation_id` and `fingerprint`, with the `scan_id` written **ahead of** any scan being created or executed | - -Only the request that newly created both its receipt and its destination can -capture proof, and it captures it from the stored prepared document. An older -or unproven receipt cannot acquire proof later through legacy enrollment or by -replaying preparation. - -Admission requires all of: a matching unconsumed reservation, completed create -and prepare stages, no queue stage, no prior decision, and no real screening -history. Any actual scan, review, finding, or model-window record, any -generation, content, processing, or provenance evidence, a marker that is -absent under an active screening policy, a recreated reservation whose -fingerprint no longer matches, or a previous queue attempt all fail closed. - -Recovery is bounded rather than automatic. The same scoped operation, actor, -and fingerprint can recover a proven pre-queue crash using the existing -operation id and a rotated conditional execution token, which is why the latch -stores an operation identity instead of a boolean. A different operation -against a consumed latch is denied. Cancellation, rejection, and errors all -retain the latch; the user-visible remedy is to cancel and re-request, which -produces a fresh receipt that is admissible on its own merits. - -`begin_scan` and `inspect_scan` persist scan-start consumption before any -effect, so a scan that races receipt preparation consumes an unverified -reservation and preparation cannot manufacture proof afterwards. Canonical -approval consumes atomically with its decision and then rechecks fresh proof -immediately before queue dispatch, so an unknown handoff never dispatches -twice. - -The exemption admits **only** the approval-to-scan handoff. It never clears or -weakens the hold, and it never admits read, download, content listing, -publication, promotion, share, or revision activation. Source authorization and -hash checks are unchanged. +Source authorization and hash checks are unchanged. Approval admits only the +processing handoff; it never admits read, download, content listing, promotion, +share or revision activation of a held destination. ## Serialization boundary @@ -272,7 +233,7 @@ for a non-owner-manager. |---|---| | `functional_tests/test_group_document_collaboration.py` | Sharing state, targets, decisions, authorization, receipts | | `functional_tests/test_group_document_publication.py` | Artifact approval, rejection, cancellation, cleanup | -| `functional_tests/test_group_document_publication_screening_bootstrap.py` | 65 cases over reservation proof, the monotonic latch, crash recovery, and every fail-closed path | +| `functional_tests/test_content_screening_upload_only_intake.py` | Since 0.261.230: published destinations get no screening marker and approval needs no screening reservation | | `functional_tests/test_public_document_payload_redaction.py` | Serialization boundary, including the roster exclusions | | `ui_tests/test_v2_group_document_collaboration.py` | 33 browser cases over the review surface | @@ -285,10 +246,8 @@ broken-access-control scanner passes on all changed backend modules. ### Deliberate limits -No legacy proof is backfilled, so a reservation predating the receipt stays -denied as unverifiable. There is no retry after an ambiguous queue attempt. -Isolated conditional-store tests do not establish live Cosmos CAS or Azure -behavior. +There is no retry after an ambiguous queue attempt. Isolated conditional-store +tests do not establish live Cosmos CAS or Azure behavior. ## Related diff --git a/docs/explanation/features/GROUP_DOCUMENT_MANAGEMENT_APIS.md b/docs/explanation/features/GROUP_DOCUMENT_MANAGEMENT_APIS.md index 92a49164c..8dc4d08de 100644 --- a/docs/explanation/features/GROUP_DOCUMENT_MANAGEMENT_APIS.md +++ b/docs/explanation/features/GROUP_DOCUMENT_MANAGEMENT_APIS.md @@ -118,10 +118,10 @@ Successful PATCH returns only a scope-bound receipt: {"message":"Group document metadata updated.","document_id":"D","group_id":"G","updated_fields":["title"],"status":"updated"} ``` -Immediate saves return HTTP 200 and `updated`. Accepted enrolled metadata rescans -return HTTP 202 and `queued`; saved metadata remains unavailable until screening -completes. Clients must validate the receipt instead of accepting arbitrary -successful HTML, objects, or message-only responses. +Saves return HTTP 200 and `updated`. Since 0.261.230 this includes screened +documents: a metadata edit applies directly and never starts a new screening +hold, so a metadata receipt is never `queued`. Clients must validate the receipt +instead of accepting arbitrary successful HTML, objects, or message-only responses. Strict writes conditionally persist the source before changing downstream projections. An ETag conflict or disappeared expected-ETag source cannot mutate diff --git a/docs/explanation/features/PUBLIC_DOCUMENT_ARTIFACT_APPROVAL_APIS.md b/docs/explanation/features/PUBLIC_DOCUMENT_ARTIFACT_APPROVAL_APIS.md index d38ced1d0..1d01eefd6 100644 --- a/docs/explanation/features/PUBLIC_DOCUMENT_ARTIFACT_APPROVAL_APIS.md +++ b/docs/explanation/features/PUBLIC_DOCUMENT_ARTIFACT_APPROVAL_APIS.md @@ -12,8 +12,8 @@ Implemented in version: **0.261.134**, tracked in Dependencies are the existing public workspace and public document containers, the shared `decide_artifact_publication` in `functions_artifact_publication.py`, -the screening bootstrap consume-latch, notifications, and Content Screening. No -new setting, container, or cloud architecture is required. +notifications, and Content Screening. No new setting, container, or cloud +architecture is required. ## Scope, and why it changed @@ -96,8 +96,7 @@ outcome: | Decision recorded, but notice or cache cleanup did not complete | `partial` | 207 | A successful approval always returns `queued` rather than `applied`, because -approving hands the document to screening and processing rather than finishing -synchronously. +approving hands the document to processing rather than finishing synchronously. Only one decision can ever commit for a given request. Repeating the decision that was already recorded returns `unchanged`. A *different* decision — for @@ -126,18 +125,16 @@ resolving the conflict silently. Cancellation requires that the actor is the original requester, so a requester can withdraw their own request without gaining authority over anyone else's. -## Screening bootstrap is reused, not reimplemented +## Screening and artifact approval -`decide_artifact_publication` is shared by the group and public paths. The -screening bootstrap consume-latch introduced for group workspaces therefore -already governs public artifact approval, and this slice reuses it unchanged. - -Public-path coverage proves the latch's rules hold here too: a reserved -`scan_id` is reservation identity rather than evidence that a scan ran; absence -of a scan row cannot distinguish "never started" from "started, errored, row -removed", so admission requires recorded proof; the latch stores the consuming -operation identity rather than a boolean; and cancellation, rejection, and -errors all retain it. +`decide_artifact_publication` is shared by the group and public paths, so the +same screening rule governs public artifact approval. Since version 0.261.230 a +published artifact is generated content: its destination is created without a +screening marker, approval hands it straight to processing, and the receipt no +longer reserves or consumes a screening scan. A destination that was already +held by screening before 0.261.230 cannot be approved; the request is refused +with 409 `publication_unavailable`, and the remedy is to cancel it and request +publication again. ## Scope-aware projection fence @@ -219,7 +216,7 @@ group and public reads modules. | Suite | Coverage | |---|---| -| `functional_tests/test_public_document_publication.py` | Approve, reject, cancel; authorization; requester-only cancellation; receipts; `etag` conflicts; cleanup; the screening bootstrap on the public path | +| `functional_tests/test_public_document_publication.py` | Approve, reject, cancel; authorization; requester-only cancellation; receipts; `etag` conflicts; cleanup; approval with no screening reservation and a held legacy destination refused | | `functional_tests/test_group_document_projection_fence.py` | Scope isolation in both directions, including a different workspace of the same kind | | `functional_tests/test_public_document_publication_predicate_single_source.py` | One definition of the publication predicate | | `functional_tests/test_document_action_hint_seam.py` | Action hints computed from policy on both projectors | diff --git a/docs/explanation/features/PUBLIC_DOCUMENT_MANAGEMENT_APIS.md b/docs/explanation/features/PUBLIC_DOCUMENT_MANAGEMENT_APIS.md index 331767393..09fe53dd7 100644 --- a/docs/explanation/features/PUBLIC_DOCUMENT_MANAGEMENT_APIS.md +++ b/docs/explanation/features/PUBLIC_DOCUMENT_MANAGEMENT_APIS.md @@ -74,7 +74,9 @@ Metadata receipt: } ``` -`status` is `updated` (200) or `queued` (202). The scope field is +`status` is `updated` (200). Since 0.261.230 a metadata edit on a screened +document applies directly and never starts a new screening hold, so metadata +receipts are no longer `queued` (202). The scope field is `public_workspace_id` where the group equivalent carries `group_id`. Incomplete propagation surfaces as a `document_propagation_incomplete` repair diff --git a/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md new file mode 100644 index 000000000..8445fd7e4 --- /dev/null +++ b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md @@ -0,0 +1,108 @@ +# Upload-Only Screening Intake Fix + +Fixed in version: **0.261.230** + +Related issue: [#1621](https://github.com/microsoft/simplechat/issues/1621) (second step). The first step is the [Workflow Run History Source Re-check Fix](WORKFLOW_RUN_HISTORY_SOURCE_RECHECK_FIX.md). + +## Issue + +Content screening was meant to check documents when they enter a workspace. With screening on, it also held documents that SimpleChat itself produced, and it held documents again whenever their metadata changed: + +- A Markdown, Word or PowerPoint file that an agent saved to a workspace was held as **Content screening pending** and went through a full scan before anyone could use it. +- A chat or workflow artifact published into a group or public workspace was created with a screening marker. Approving it had to pass a receipt-bound screening reservation, and a request whose reservation couldn't be proven was refused with "This screening reservation cannot start publication." +- Editing a cleared document's title, abstract, keywords or tags started a new scan and held the document again. The group and public metadata APIs answered `queued` (202) instead of `updated`. +- Metadata the model generated for a newly released upload did the same: the document was held again until its metadata was re-inspected. +- The release proof included a fingerprint of the document's metadata, so a metadata change that didn't go through a rescan made a cleared document unavailable. + +## Root Cause + +- **Intake.** `document_requires_screening` enrolled every new document while screening was on and the effective policy had checks. `create_document` chose the initial screening marker without knowing who created the document, so generated documents were enrolled like uploads. +- **Publication.** Because publication destinations were enrolled, approval needed proof that it was starting the destination's first scan. `functions_artifact_publication.py` recorded a `screening_reservation` in the receipt, and `begin_scan`/`inspect_scan` consumed it through `_consume_publication_reservation` and `consume_artifact_publication_screening_scan`. +- **Metadata.** `update_document` sent any change to a content metadata field of a screened document to `queue_metadata_rescan`, which began a new scan and held the document. Model-generated metadata is written through `update_document`, so it took the same path. +- **Release proof.** `_require_release_proof`, `finalize_publication_checkpoint` and the scan-job publication match compared a `metadata_fingerprint` recorded at release with the document's current metadata, so any metadata change invalidated the release. + +Under the agreed upload-only model, screening checks only documents that enter a workspace by a user upload or File Sync, generated content is never checked, and metadata edits are not screened. + +## Technical Details + +Files modified: + +- `application/single_app/content_screening/contracts.py` +- `application/single_app/content_screening/service.py` +- `application/single_app/content_screening/jobs.py` +- `application/single_app/content_screening/access.py` +- `application/single_app/functions_documents.py` +- `application/single_app/functions_simplechat_operations.py` +- `application/single_app/functions_artifact_publication.py` +- `application/single_app/functions_artifact_publication_readiness.py` +- `application/single_app/functions_group_document_publication.py` +- `application/single_app/functions_public_document_publication.py` +- `application/single_app/functions_group_document_management.py` +- `application/single_app/functions_public_document_management.py` +- `application/single_app/config.py` +- `functional_tests/test_content_screening_upload_only_intake.py` (new) +- `functional_tests/test_content_screening_jobs.py` +- `functional_tests/test_content_screening_pipeline.py` +- `functional_tests/test_group_document_management.py` +- `functional_tests/test_public_document_publication.py` +- `functional_tests/test_group_document_publication_screening_bootstrap.py` (removed) + +### Marking generated documents + +- `contracts.py` defines `SCREENING_EXEMPTION_FIELD` (`screening_exemption`), `generated_screening_exemption()` (`{"reason": "generated", "version": 1}`) and `is_generated_screening_exempt()`, which accepts only that exact value. +- `create_document` takes a server-only `screening_exemption` argument. It is validated before any earlier revision is archived and stored on the new version before `initial_document_marker` runs. It is not carried forward to later versions. +- `_upload_generated_document_for_current_user` (used by the Markdown, generated-document, Word and PowerPoint agent uploads) and `_publish_generated_chat_artifact_for_user` pass the exemption. These are the only `create_document` callers that do. User uploads, chat uploads saved to a workspace and File Sync don't, so they are screened as before. The provenance `origin` field is not used, because a `chat` origin also marks a user's chat upload. +- The name starts with `screening`, so `reject_screening_fields` rejects it in any client request to the document APIs, and `is_public_document_field` keeps it out of every document response. `update_document` refuses every `screening_*` keyword, so no update path can set or change it. The upload routes build their records from explicit arguments and never copy client fields. + +### Skipping screening at intake + +- `document_requires_screening` returns `False` for an exempt document that has no `content_screening` marker. An existing marker still wins. `initial_document_marker`, `prepare_document_upload`, `process_document_upload_background` and `process_document_reprocess_extraction_background` all follow it. +- `begin_scan` and the scan-job hold step raise `ScreeningNotRequiredError` (`screening_not_required`, a conflict error) for an exempt document without a marker, so scan jobs record it as skipped and never write a marker. + +### Removing the publication screening reservation + +- Removed `_consume_publication_reservation`, `consume_artifact_publication_screening_scan`, `_enroll_screening_reservation`, `_reservation_has_screening_history`, `publication_screening_reservation`, the `screening_reservation`/`screening_reservation_consumption` receipt fields and the bootstrap retry in the group and public publication adapters. +- `_publication_destination_admits_approval` admits a destination with no screening marker, and one whose screening release is available. A destination held by screening is refused with `PUBLICATION_SCREENING_HOLD_MESSAGE`, which tells the requester to cancel and request publication again. Only destinations created before this version can carry a marker. +- `inspect_publication_readiness` already reports a marker-free destination as `screening: not_required` and still confirms access to the destination before it reports index readiness. + +### Metadata + +- `update_document` applies metadata edits to a screened document directly, including model-generated metadata from `process_metadata_extraction_background`. `queue_metadata_rescan` is replaced by `validate_screened_metadata_update`, which keeps the rule that a rename cannot change a screened file's extension. +- Chunk metadata (title, authors, file name, classification and tags) is now propagated for a screened document while its release is available. Held documents have no released chunks; publication rebuilds them. +- `propagate_tags_to_blob_metadata` no longer writes to a screened document's blob. The release pins that blob by ETag and content hash, so rewriting its metadata would have made the released file unreadable. +- `_require_release_proof`, `finalize_publication_checkpoint` and `_publication_matches_document` no longer compare the metadata fingerprint. The release still records it. The scan-job reconciliation no longer has a special case for metadata-triggered child scans, because there are none. +- `update_group_document_metadata` and `update_public_document_metadata` always return `updated`, so their routes answer 200. + +### Impact + +- Generated documents are processed and searchable as soon as processing finishes, whether or not screening is on. +- Users can edit metadata of released documents without losing access to them. +- Existing data is not migrated. Documents keep their current screening state, nothing already screened or held is released, and a publication destination held by screening before this version must be requested again. + +### Known limitations + +- Release publishes the metadata captured from the inspected copy. A metadata edit made through the API while a document is still held is replaced by that copy when the document is released. The workspace interfaces don't offer metadata edits on held documents. +- The V2 explorer still has an unused branch for a `queued` metadata receipt. It is no longer reachable. + +## Validation + +`functional_tests/test_content_screening_upload_only_intake.py` (127 cases) runs the real `create_document`, `update_document`, metadata extraction, upload dispatch, generated-upload, scan-job, publication and document-guard code against in-memory stores, with screening on and an active policy: + +- Generated and published documents get no marker and keep the exemption; user uploads, chat uploads and File Sync documents in all three scopes still get a `pending_scan` marker. The exemption belongs to one version, a malformed exemption is refused before anything is written, and an existing marker still wins. +- `prepare_document_upload` and the upload dispatcher process a generated document without screening. An agent upload in a personal or group workspace creates an unscreened document. +- Only the two generated creation sites pass the exemption; the upload and File Sync sites don't. +- Every guarded document mutation route rejects a client exemption in JSON and form bodies, `update_document` refuses it, and responses never include it. +- A scan job skips a generated document with `screening_not_required` and still scans an uploaded one; `begin_scan` refuses a generated document. +- Editing metadata after release keeps the release proof, a rename can't change a screened file's extension, and model-generated metadata applies to a released group document without a hold, updates its chunks and leaves its blob alone. +- A group publication creates an unscreened destination, records no screening reservation in its receipt, and is approved and queued without a scan. +- The reservation and metadata-rescan functions no longer exist, and the screening and readiness modules still import without network access or application owners. + +With the application changes reverted and only the test changes applied, 22 cases in that file fail and 3 cannot set up; the rest are the existing client guards, readiness and import checks, which pass either way. + +Existing tests that asserted the removed behavior were rewritten: + +- `test_content_screening_jobs.py`: metadata-triggered child scans became generated metadata that applies without a new hold, including crash recovery; a title or publication-metadata change no longer blocks an interrupted release from finishing. +- `test_content_screening_pipeline.py`: a metadata edit keeps a release, recovery finalizes a release after a metadata edit, and only the source-format rule remains. +- `test_group_document_management.py`: a screened metadata edit returns `updated` and keeps the release; a screened tag edit updates chunks but not the release blob. +- `test_public_document_publication.py`: approval records no screening reservation, and a held legacy destination blocks approval. +- `test_group_document_publication_screening_bootstrap.py` tested only the removed reservation and was removed. Its import check moved to the new file. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index fd9644408..859f59eaa 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,23 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/features) and [Fixes by Version](https://github.com/microsoft/simplechat/tree/main/docs/explanation/fixes). +### **(v0.261.230)** + +#### Breaking Changes + +* **Generated Documents Are No Longer Screened** + * Content screening now checks only documents that enter a workspace by a user upload (personal, group or public, including a file attached in chat and saved to a workspace) or File Sync. Files the SimpleChat agent action saves to a workspace and artifacts published into a workspace get no screening marker, are processed normally while screening is on, and are skipped by scan jobs. + * Each generated document version carries a server-managed `screening_exemption` field that is set when the version is created. Clients can't set it: the document APIs reject any `screening` field, no update path accepts one, and responses never include it. A document that already has a screening marker stays screened. + * Publishing an artifact to a group or public workspace no longer reserves a screening scan for its destination, so approval hands it straight to processing. A destination that was held by screening before this version can't be approved; cancel the request and publish the artifact again. + * **Migration**: None. Existing documents keep their screening state, and nothing already screened or held is released. + * (Ref: #1621, `content_screening/contracts.py`, `content_screening/service.py`, `content_screening/jobs.py`, `functions_documents.py`, `functions_simplechat_operations.py`, `functions_artifact_publication.py`, `functions_artifact_publication_readiness.py`, [Upload-Only Screening Intake Fix](fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md)) + +* **Metadata Edits No Longer Hold Screened Documents Again** + * Editing a released document's title, abstract, keywords, authors, classification or tags now applies directly. The document stays available and no new scan starts, so the group and public document metadata APIs return `updated` (200) instead of `queued` (202). + * Metadata the model generates after a document is released also applies directly, without a new hold. + * A rename still can't change a screened file's extension. Tag edits on a screened document update the document and its search chunks but no longer rewrite the released file's blob metadata, so downloads of that file keep working. + * (Ref: #1621, `functions_documents.update_document`, `content_screening/access.py`, `functions_group_document_management.py`, `functions_public_document_management.py`, [Upload-Only Screening Intake Fix](fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md)) + ### **(v0.261.229)** #### Bug Fixes diff --git a/docs/guides/review-screened-documents.md b/docs/guides/review-screened-documents.md index 28caa3466..4bf7ff572 100644 --- a/docs/guides/review-screened-documents.md +++ b/docs/guides/review-screened-documents.md @@ -4,17 +4,23 @@ title: "Screen and review workspace documents" description: "Inspect sensitive or manipulative extracted content, keep it out of knowledge use, and release only a reviewed version." section: "Guides" audience: user -version: "0.261.127" +version: "0.261.230" --- ## What this does -Content screening checks workspace knowledge for configured PII, patterns, values, and optional model-evaluated criteria. A finding holds the whole document so a clean early page cannot be used while another page awaits review. +Content screening checks documents that people upload to a workspace, or that File Sync brings in, for configured PII, patterns, values, and optional model-evaluated criteria. A finding holds the whole document so a clean early page cannot be used while another page awaits review. **Implemented in version: 0.261.106**, tracked in `application\single_app\config.py`. Use it when a document might contain sensitive data or instructions aimed at an AI rather than a human reader. Text can enter extraction even when its appearance makes it hard to notice in the original. +### What isn't screened + +Since **0.261.230**, documents that SimpleChat generates aren't screened. That includes files an agent saves to a workspace and chat or workflow artifacts published to a workspace. Files you attach in chat that are saved to a workspace are uploads, so they're still screened. + +Editing a document's title, abstract, keywords, tags or other metadata isn't screened either, and doesn't hold a document that was already released. The same applies to metadata the model generates for a document after it's released. You still can't rename a screened file to a different file type. + ## Configure an appropriate baseline An administrator must configure Enhanced Citations and its storage account before enabling screening. The capability is off by default. @@ -63,7 +69,7 @@ Choose the documents or workspaces to inspect. Only administrators can start an In V2, **Screening scans** in the Documents section scans the workspace you're in: your own documents in My Workspace, and, from version **0.261.174**, a group's documents for its Owner, Admins and DocumentManagers. -Use the workload/progress view to distinguish queued, running, completed, flagged, incomplete, and failed documents. Queued documents remain in their previous state until their scan starts. Started documents remain held if a check cannot finish. +Use the workload/progress view to distinguish queued, running, completed, flagged, incomplete, and failed documents. Queued documents remain in their previous state until their scan starts. Started documents remain held if a check cannot finish. Documents SimpleChat generated are skipped rather than scanned. Do not interpret a completed enumeration or a model timeout as evidence that every document passed. Correct the reported source, model, policy, or storage problem and resume or retry the work. Cancelling a job does not release documents that are already held. @@ -109,6 +115,8 @@ Review original files as potentially untrusted content. Do not follow links or i | The document has a warning after approval | It was approved with flags; the warning is intentional. | | An original is missing | Source-backed inspection or re-extraction may be unavailable. A table schema summary does not cover the underlying cell data. | | Scanning was disabled but a document is still held | Disabling future scans does not approve existing held content. | +| A file an agent saved, or an artifact you published, has no screening status | By design: documents SimpleChat generates aren't screened. | +| A published artifact waiting for approval can't be approved because it's held by screening | It was requested before generated content stopped being screened. Cancel the request and publish the artifact again. | | An older attachment or tool result is unavailable in conversation history | Its source is held, or that old result cannot prove it used the currently approved revision. Complete review and run a new query against the approved document. | | Another person cannot open the evidence | Review access is scoped. Administrative scan permission or ordinary shared-read access is not sufficient. | diff --git a/functional_tests/test_content_screening_jobs.py b/functional_tests/test_content_screening_jobs.py index 00958ddc5..c43ad1da3 100644 --- a/functional_tests/test_content_screening_jobs.py +++ b/functional_tests/test_content_screening_jobs.py @@ -1,8 +1,9 @@ # test_content_screening_jobs.py """ Behavioral tests for durable content screening jobs and crash recovery. -Version: 0.261.106 +Version: 0.261.230 Implemented in: 0.261.106 +Generated metadata applies without a child scan implemented in: 0.261.230 Uses fake Cosmos/Blob clients, real service/detector/CAS methods, a controlled clock, and external processor doubles. No live Azure resources are read. @@ -1569,10 +1570,10 @@ def concurrent_source_change(repository, current, **updates): @pytest.mark.parametrize("changed", [ - "blob_path", "blob_etag", "source_etag", "file_size", "title", + "blob_path", "blob_etag", "source_etag", "file_size", "marker_policy", "marker_content", "marker_canonical", "marker_scan", "marker_job", "marker_state", "policy", "policy_fingerprint", "content_fingerprint", "units_ref", "publication", "coverage_complete", - "publication_metadata", "publication_blob", "scan_state", + "publication_blob", "scan_state", ]) def test_publication_retry_never_waives_an_unbound_source_change(service_runtime, monkeypatch, changed): runtime = service_runtime @@ -1581,7 +1582,7 @@ def test_publication_retry_never_waives_an_unbound_source_change(service_runtime item = interrupt_publication(runtime, monkeypatch, job) document = repository.read_document(subject) current = repository.get_scan(scan["id"]) - if changed in {"blob_path", "blob_etag", "source_etag", "file_size", "title"}: + if changed in {"blob_path", "blob_etag", "source_etag", "file_size"}: repository.update_document(subject, { changed: 123456 if changed == "file_size" else "unrelated-change", }, etag=document["_etag"]) @@ -1597,10 +1598,9 @@ def test_publication_retry_never_waives_an_unbound_source_change(service_runtime repository.update_document(subject, { "content_screening": {**document["content_screening"], field: value}, }, etag=document["_etag"]) - elif changed in {"publication_metadata", "publication_blob"}: - field = "metadata_fingerprint" if changed == "publication_metadata" else "active_blob" + elif changed == "publication_blob": repository.replace({ - **current, "publication": {**current["publication"], field: hash_payload("different-projection")}, + **current, "publication": {**current["publication"], "active_blob": hash_payload("different-projection")}, }, current["_etag"]) else: field, value = { @@ -1624,6 +1624,29 @@ def test_publication_retry_never_waives_an_unbound_source_change(service_runtime assert repository.read_document(subject) == before +@pytest.mark.parametrize("changed", ["title", "publication_metadata"]) +def test_publication_retry_finishes_a_release_after_a_metadata_change(service_runtime, monkeypatch, changed): + runtime = service_runtime + repository = runtime.repository + subject, scan, job = stage_scan(runtime) + interrupt_publication(runtime, monkeypatch, job) + document = repository.read_document(subject) + current = repository.get_scan(scan["id"]) + if changed == "title": + repository.update_document(subject, {"title": "Edited after release"}, etag=document["_etag"]) + else: + repository.replace({ + **current, "publication": {**current["publication"], "metadata_fingerprint": hash_payload("other-metadata")}, + }, current["_etag"]) + result = jobs.run_scan_job(job["id"], repository=repository) + assert result["status"] == "completed" and result["counts"]["completed"] == 1 + assert len(runtime.published) == len(runtime.inspections) == repository.count("scan") == 1 + assert repository.get_scan(scan["id"])["state"] == "cleared" + released = repository.read_document(subject) + assert document_is_available(released) + access._require_release_proof(released, repository.container) + + def test_saved_scan_proof_can_finalize_without_an_item_projection_checkpoint(service_runtime, monkeypatch): runtime = service_runtime repository = runtime.repository @@ -1694,28 +1717,31 @@ def test_default_processor_rejects_terminal_scans_with_pending_document_markers( assert repository.read_document(subject)["content_screening"]["state"] == "pending_scan" -def test_publication_resume_finishes_parent_before_enqueuing_metadata_child(service_runtime, monkeypatch): +def test_publication_resume_applies_generated_metadata_without_a_new_hold(service_runtime, monkeypatch): runtime = service_runtime repository = runtime.repository runtime.settings["enable_extract_meta_data"] = True subject, scan, job = stage_scan(runtime, trigger="upload") item = interrupt_publication(runtime, monkeypatch, job) - children = [] + generated = [] def extract_metadata(document_id, actor_id, **kwargs): assert jobs.PROCESSOR_CONTEXT.get() is None assert repository.get(item["id"], job["id"])["status"] == "completed" - children.append(service.queue_metadata_rescan( - repository.read_document(subject), {"title": "Generated title"}, actor_id, repository=repository, - )) + document = repository.read_document(subject) + # Model-generated metadata is written directly, the same way update_document applies it. + generated.append(repository.update_document(subject, {"title": "Generated title"}, etag=document["_etag"])) runtime.helpers.process_metadata_extraction_background = extract_metadata result = jobs.run_scan_job(job["id"], repository=repository) assert result["status"] == "completed" and result["counts"]["completed"] == 1 assert repository.get_scan(scan["id"])["state"] == "cleared" - marker = repository.read_document(subject)["content_screening"] - assert len(children) == 1 and marker["scan_id"] == children[0]["id"] and marker["state"] == "pending_scan" - assert not runtime.notices and repository.count("scan") == repository.count("job") == 2 + document = repository.read_document(subject) + assert len(generated) == 1 and document["title"] == "Generated title" + assert document["content_screening"]["scan_id"] == scan["id"] and document_is_available(document) + access._require_release_proof(document, repository.container) + assert len(runtime.notices) == 1 + assert repository.count("scan") == repository.count("job") == 1 def test_real_completion_failure_does_not_rehold_and_recovers_while_disabled(service_runtime): @@ -1848,68 +1874,63 @@ def crash(*args, **kwargs): assert "content_screening" not in repository.read_document(Subject.from_dict(pending["subject"])) -def test_real_metadata_child_starts_only_after_parent_item_is_finished(service_runtime): +def test_generated_metadata_applies_after_the_parent_item_finishes(service_runtime): runtime = service_runtime repository = runtime.repository runtime.settings["enable_extract_meta_data"] = True subject, scan, job = stage_scan(runtime, trigger="upload") - children = [] def extract_metadata(document_id, actor_id, **kwargs): assert jobs.PROCESSOR_CONTEXT.get() is None assert repository.query("work_item", filters={"scan_id": scan["id"]})["items"][0]["status"] == "completed" document = repository.read_document(subject) - children.append(service.queue_metadata_rescan( - document, {"title": "Generated title"}, actor_id, repository=repository, - )) + repository.update_document(subject, { + "title": "Generated title", "abstract": "Generated abstract", + }, etag=document["_etag"]) runtime.helpers.process_metadata_extraction_background = extract_metadata parent = jobs.run_scan_job(job["id"], repository=repository) assert parent["status"] == "completed" and parent["counts"]["completed"] == 1 - child = children[0] - marker = repository.read_document(subject)["content_screening"] - assert marker["scan_id"] == child["id"] and marker["state"] == "pending_scan" - assert repository.get_scan(scan["id"])["state"] == "cleared" - assert not runtime.notices - child_job = repository.query("job", filters={"status": "queued"})["items"][0] - completed_child = jobs.run_scan_job(child_job["id"], repository=repository) - assert completed_child["status"] == "completed" - assert repository.read_document(subject)["content_screening"]["scan_id"] == child["id"] - assert len(runtime.published) == 2 and len(runtime.notices) == len(children) == 1 - assert runtime.notices[0]["metadata"]["screening_scan_id"] == child["id"] - assert repository.count("scan") == repository.count("job") == 2 + document = repository.read_document(subject) + assert document["title"] == "Generated title" and document_is_available(document) + assert document["content_screening"]["scan_id"] == scan["id"] + assert document["content_screening"]["metadata_generated"] is True + access._require_release_proof(document, repository.container) + assert len(runtime.published) == len(runtime.notices) == 1 + assert runtime.notices[0]["metadata"]["screening_scan_id"] == scan["id"] + assert repository.count("scan") == repository.count("job") == 1 jobs.check_due_scan_jobs_once(repository=repository) - assert repository.read_document(subject)["content_screening"]["scan_id"] == child["id"] + assert repository.count("scan") == 1 assert document_is_available(repository.read_document(subject)) -def test_metadata_enqueue_crash_recovers_parent_without_reholding_child(service_runtime): +def test_crash_after_generated_metadata_recovers_without_a_new_hold(service_runtime): runtime = service_runtime repository = runtime.repository runtime.settings["enable_extract_meta_data"] = True subject, scan, job = stage_scan(runtime, trigger="upload") - children = [] def extract_metadata(document_id, actor_id, **kwargs): - children.append(service.queue_metadata_rescan( - repository.read_document(subject), {"title": "Generated title"}, actor_id, repository=repository, - )) - raise SystemExit("simulated worker loss after metadata child enqueue") + document = repository.read_document(subject) + repository.update_document(subject, {"title": "Generated title"}, etag=document["_etag"]) + raise SystemExit("simulated worker loss after generated metadata") runtime.helpers.process_metadata_extraction_background = extract_metadata with pytest.raises(SystemExit): jobs.run_scan_job(job["id"], repository=repository) - child_document = repository.read_document(subject) - assert child_document["content_screening"]["scan_id"] == children[0]["id"] + generated = repository.read_document(subject) + assert generated["title"] == "Generated title" and document_is_available(generated) assert repository.get_scan(scan["id"])["postprocess_pending"] runtime.clock["now"] += timedelta(seconds=jobs.LEASE_SECONDS + 1) runtime.settings["enable_content_screening"] = False jobs.check_due_scan_jobs_once(repository=repository) assert repository.get(job["id"], job["id"])["status"] == "completed" - assert repository.read_document(subject) == child_document assert repository.get_scan(scan["id"])["postprocess_pending"] is False - assert len(runtime.published) == 1 and not runtime.notices - assert repository.count("scan") == repository.count("job") == 2 + current = repository.read_document(subject) + assert current["title"] == "Generated title" and document_is_available(current) + access._require_release_proof(current, repository.container) + assert len(runtime.published) == len(runtime.notices) == 1 + assert repository.count("scan") == repository.count("job") == 1 def test_approved_with_flags_completion_retries_without_reopening_review(service_runtime): diff --git a/functional_tests/test_content_screening_pipeline.py b/functional_tests/test_content_screening_pipeline.py index 99d24d1ab..95ffdca03 100644 --- a/functional_tests/test_content_screening_pipeline.py +++ b/functional_tests/test_content_screening_pipeline.py @@ -1,11 +1,12 @@ # test_content_screening_pipeline.py """ Functional integration tests for workspace admission and reviewed publication. -Version: 0.261.189 +Version: 0.261.230 Implemented in: 0.261.106 The exec namespace carries ONENOTE_EXTENSIONS, which the upload path reads from 0.261.189: 0.261.189 Enabled-empty upload admission implemented in: 0.261.114 Publication processing evidence implemented in: 0.261.118 +Metadata edits keep a release (no metadata rescan) implemented in: 0.261.230 Runs the real durable job, scanner, repository, private storage, TXT extraction, and publication services against fake Azure boundaries. No live data is used. @@ -41,6 +42,7 @@ ScreeningChecksRequiredError, ScreeningConflictError, ScreeningError, + ScreeningValidationError, Subject, document_is_available, hash_payload, @@ -574,26 +576,31 @@ def test_restored_clear_marker_without_release_proof_is_not_available(pipeline, access.assert_document_available("document", "owner") -def test_unscreened_metadata_cannot_borrow_previous_clearance(pipeline, tmp_path): +def test_metadata_edits_after_release_keep_the_clearance(pipeline, tmp_path): seed(pipeline) upload(pipeline, tmp_path, "Ordinary evidence") document = pipeline.repository.document_container("personal").read_item("document", "document") - document["abstract"] = "PRIVATE_CANARY" + document["abstract"] = "An edited abstract" + document["tags"] = ["edited"] pipeline.repository.document_container("personal").replace_item( item="document", body=document, etag=document["_etag"], match_condition=MatchConditions.IfNotModified, ) - with pytest.raises(DocumentHeldError): - access.assert_document_available("document", "owner") + available = access.assert_document_available("document", "owner") + assert available["abstract"] == "An edited abstract" + assert access.read_available_document_bytes("document", "owner")[1] == b"Ordinary evidence" + assert len(pipeline.repository.query("scan")["items"]) == 1 -def test_initial_metadata_changes_do_not_enqueue_a_sourceless_rescan(pipeline): +def test_metadata_validation_keeps_only_the_screened_source_format(pipeline): document = seed(pipeline) - assert service.queue_metadata_rescan(document, {"tags": ["intake"]}, "owner", repository=pipeline.repository) is None + assert service.validate_screened_metadata_update(document, {"tags": ["intake"], "file_name": "renamed.txt"}) is None + with pytest.raises(ScreeningValidationError): + service.validate_screened_metadata_update(document, {"file_name": "renamed.pdf"}) + unscreened = {key: value for key, value in document.items() if key != SCREENING_FIELD} + assert service.validate_screened_metadata_update(unscreened, {"file_name": "renamed.pdf"}) is None assert not pipeline.repository.query("job")["items"] - current = pipeline.repository.read_document(Subject("personal", "owner", "document", "1")) - assert current["tags"] == ["intake"] - assert current[SCREENING_FIELD]["scan_id"] == document[SCREENING_FIELD]["scan_id"] + assert not pipeline.repository.query("scan")["items"] def test_stale_worker_failure_does_not_clear_another_workers_lease(pipeline): @@ -750,15 +757,15 @@ def test_recovery_finalizes_only_the_already_committed_publication(pipeline, tmp assert access.read_available_document_bytes("document", "owner")[1] == b"Complete inspected text" -def test_recovery_does_not_invent_proof_for_changed_metadata(pipeline, tmp_path, monkeypatch): +def test_recovery_finalizes_a_release_after_a_metadata_edit(pipeline, tmp_path, monkeypatch): scan = interrupt_after_document_release(pipeline, tmp_path, monkeypatch) subject = Subject("personal", "owner", "document", "1") document = pipeline.repository.read_document(subject) - pipeline.repository.update_document(subject, {"abstract": "UNAPPROVED"}, etag=document["_etag"]) - with pytest.raises(ScreeningConflictError): - service.finalize_publication_checkpoint(scan["id"], repository=pipeline.repository) - with pytest.raises(DocumentHeldError): - access.assert_document_available("document", "owner") + pipeline.repository.update_document(subject, {"abstract": "Edited after release"}, etag=document["_etag"]) + finished = service.finalize_publication_checkpoint(scan["id"], repository=pipeline.repository) + assert finished["state"] == "cleared" + assert access.assert_document_available("document", "owner")["abstract"] == "Edited after release" + assert access.read_available_document_bytes("document", "owner")[1] == b"Complete inspected text" def test_service_model_resume_reuses_durable_validated_windows(pipeline, tmp_path): diff --git a/functional_tests/test_content_screening_upload_only_intake.py b/functional_tests/test_content_screening_upload_only_intake.py new file mode 100644 index 000000000..6b85ed306 --- /dev/null +++ b/functional_tests/test_content_screening_upload_only_intake.py @@ -0,0 +1,702 @@ +# test_content_screening_upload_only_intake.py +""" +Functional tests for upload-only content screening intake. +Version: 0.261.230 +Implemented in: 0.261.230 + +This test ensures that, with content screening on and an active policy, documents +SimpleChat generates or publishes into a workspace never receive a screening marker, +while user uploads, chat uploads and File Sync still do. It also ensures that clients +cannot set the server-managed exemption, that editing metadata keeps a cleared document +available, that model-generated metadata applies without a new hold, and that the +publication screening reservation is gone. Refs #1621. + +Runs the real create_document, update_document, metadata extraction, upload dispatch, +generated-upload, scan-job, publication and document-guard code against in-memory +stores. No Azure resources, models or application startup are used. +""" + +import ast +from copy import deepcopy +from datetime import datetime, timezone +import logging +import os +from pathlib import Path +import re +import subprocess +import sys +from types import SimpleNamespace +from typing import Any, Dict +import uuid + +import pytest + +APP_ROOT = Path(__file__).resolve().parents[1] / "application" / "single_app" +if str(APP_ROOT) not in sys.path: + sys.path.insert(0, str(APP_ROOT)) + +from content_screening import access, contracts, jobs, service # noqa: E402 +from content_screening.contracts import ( # noqa: E402 + SCREENING_FIELD, + DocumentHeldError, + ScreeningConflictError, + ScreeningValidationError, + Subject, + content_fingerprint, + document_is_available, + hash_payload, + subject_from_document, +) +from content_screening.policies import default_policy # noqa: E402 +from content_screening.repository import ScreeningRepository # noqa: E402 +import functions_artifact_publication_readiness as readiness # noqa: E402 +import functions_document_provenance as provenance # noqa: E402 +from test_content_screening_jobs import add_document, create_job, runtime, service_runtime # noqa: E402,F401 +from test_content_screening_persistence import FakeCosmos # noqa: E402 +from test_document_provenance_versions_and_guards import ( # noqa: E402,F401 + MUTATING_ROUTES, + load_functions, + mutation_client, +) +from test_group_document_management import management # noqa: E402,F401 +from test_group_document_publication import ( # noqa: E402,F401 + REQUESTER, + assert_receipt, + decide, + publication, + publication_modules, + receipt, + sharing, + submit, +) +from test_group_document_read_apis import environment # noqa: E402,F401 +from test_support.agent_delegation import execute_functions # noqa: E402 +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +GENERATED = {"reason": "generated", "version": 1} +SCOPES = { + "personal": {}, + "group": {"group_id": "group-1"}, + "public": {"public_workspace_id": "public-1"}, +} +ACTIVE_POLICY_RULE = { + "id": "restricted", "name": "Restricted", "type": "literal", "enabled": True, + "severity": "high", "category": "sensitive", "values": ["PRIVATE_CANARY"], +} + + +def generated_exemption(): + # Resolved at call time so the suite reports per-test failures without the feature. + return contracts.generated_screening_exemption() + + +def test_version_supports_upload_only_intake(): + assert_app_version_at_least("0.261.230") + + +# --- Exemption contract ----------------------------------------------------------------- + + +def test_the_exemption_is_an_exact_server_marker(): + first = generated_exemption() + first["version"] = 2 + assert generated_exemption() == GENERATED + assert contracts.SCREENING_EXEMPTION_FIELD == "screening_exemption" + assert contracts.is_generated_screening_exempt({"screening_exemption": GENERATED}) + for near_match in ( + {"reason": "generated", "version": True}, {"reason": "generated", "version": "1"}, + {"reason": "Generated", "version": 1}, {"reason": "generated"}, + {"reason": "generated", "version": 1, "scope": "all"}, ["generated", 1], None, + ): + assert not contracts.is_generated_screening_exempt({"screening_exemption": near_match}), near_match + assert not contracts.is_generated_screening_exempt(None) + + +@pytest.mark.parametrize("payload", [ + {"screening_exemption": GENERATED}, + {"screeningExemption": GENERATED}, + {"Screening-Exemption": "generated"}, + {"metadata": {"screening_exemption": GENERATED}}, + {"documents": [{"id": "doc-1", "SCREENING_EXEMPTION": GENERATED}]}, +]) +def test_clients_cannot_send_the_exemption_in_any_document_payload(payload): + with pytest.raises(ScreeningValidationError): + access.reject_screening_fields(payload) + + +@pytest.mark.parametrize("route", MUTATING_ROUTES, ids=lambda route: f"{route[2]} {route[1]}") +def test_every_guarded_document_mutation_route_rejects_a_client_exemption(mutation_client, route): + path, method = mutation_client.paths[route], route[2] + mutation_client.reached.clear() + for body in ({"title": "Report", "screening_exemption": GENERATED}, {"meta": {"screeningExemption": GENERATED}}): + response = mutation_client.client.open(path, method=method, json=body) + assert response.status_code == 400, (route, body) + assert response.get_json()["error_code"] == ScreeningValidationError.code + response = mutation_client.client.open(path, method=method, data={"screening_exemption": "generated"}) + assert response.status_code == 400, route + assert mutation_client.reached == [] + + +def test_the_exemption_is_never_echoed_to_clients(): + stored = {"id": "doc-1", "file_name": "report.md", "user_id": "user-1", "screening_exemption": GENERATED} + assert not access.is_public_document_field("screening_exemption") + assert "screening_exemption" not in access.public_document_payload(stored) + + +class ReachedStorage(Exception): + """Raised by the operation guard to prove a call got past the field guards.""" + + +def test_no_update_path_can_set_or_change_the_exemption(): + namespace = load_functions("functions_documents.py", {"update_document"}, { + "SCREENING_FIELD": SCREENING_FIELD, "ScreeningValidationError": ScreeningValidationError, + "ORIGIN_FIELD_NAMES": provenance.ORIGIN_FIELD_NAMES, "DocumentOriginError": provenance.DocumentOriginError, + }) + reached = [] + + def guard(): + reached.append(True) + raise ReachedStorage() + + for field in ("screening_exemption", "screening_state"): + with pytest.raises(ScreeningValidationError): + namespace["update_document"](document_id="doc-1", user_id="user-1", operation_guard=guard, **{field: GENERATED}) + assert reached == [] + with pytest.raises(ReachedStorage): + namespace["update_document"](document_id="doc-1", user_id="user-1", operation_guard=guard, title="Report") + + +# --- Intake: the real create_document --------------------------------------------------- + + +class DocumentStore: + """A scope-filtered stand-in for one Cosmos documents container.""" + + def __init__(self): + self.records = {} + + def query_items(self, query, parameters, enable_cross_partition_query=True): + values = {item["name"]: item["value"] for item in parameters} + fields = { + "@file_name": "file_name", "@user_id": "user_id", "@group_id": "group_id", + "@public_workspace_id": "public_workspace_id", + } + return [ + deepcopy(record) for record in self.records.values() + if all(record.get(field) == values[name] for name, field in fields.items() if name in values) + ] + + def upsert(self, document): + self.records[document["id"]] = deepcopy(document) + + +@pytest.fixture +def screening_on(monkeypatch): + repository = ScreeningRepository(FakeCosmos(), {scope: FakeCosmos(partition_field="id") for scope in SCOPES}) + policy = default_policy() + policy.update(enabled=True, rules=[dict(ACTIVE_POLICY_RULE)]) + repository.save_policy("global", "global", policy, "administrator") + settings = { + "enable_content_screening": True, "enable_enhanced_citations": True, + "enable_content_screening_workspace_uploads": True, + } + monkeypatch.setattr(service, "_repository", lambda value=None: value if value is not None else repository) + monkeypatch.setattr(service, "_settings", lambda value=None: value if value is not None else settings) + monkeypatch.setattr(service, "_log", lambda *args, **kwargs: None) + return SimpleNamespace(repository=repository, settings=settings) + + +@pytest.fixture +def intake(screening_on): + stores = {scope: DocumentStore() for scope in SCOPES} + namespace = { + "datetime": datetime, "timezone": timezone, "logging": logging, "re": re, + "SCREENING_FIELD": SCREENING_FIELD, + "SCREENING_EXEMPTION_FIELD": getattr(contracts, "SCREENING_EXEMPTION_FIELD", "screening_exemption"), + "is_generated_screening_exempt": getattr(contracts, "is_generated_screening_exempt", lambda document: False), + "ScreeningValidationError": ScreeningValidationError, + "cosmos_user_documents_container": stores["personal"], + "cosmos_group_documents_container": stores["group"], + "cosmos_public_documents_container": stores["public"], + "require_xsd_ingestion_capability": lambda *args, **kwargs: None, + "set_document_chunk_visibility": lambda document, active: None, + "_upsert_document_and_sync_access_index": lambda container, document, operation: container.upsert(document), + "_get_blob_container_name": lambda **kwargs: "documents", + "apply_document_provenance": provenance.apply_document_provenance, + "validate_origin": provenance.validate_origin, + "initial_document_marker": service.initial_document_marker, + "add_file_task_to_file_processing_log": lambda *args, **kwargs: None, + "log_event": lambda *args, **kwargs: None, + } + load_functions("functions_documents.py", { + "create_document", "_build_carried_forward_metadata", "_document_revision_sort_key", "_safe_int", + "ensure_list", + }, namespace) + + def create(scope, document_id, *, file_name="report.md", **values): + namespace["create_document"]( + file_name=file_name, user_id="user-1", document_id=document_id, num_file_chunks=0, + status="Queued for processing", **SCOPES[scope], **values, + ) + return deepcopy(stores[scope].records[document_id]) + + return SimpleNamespace( + create=create, stores=stores, create_document=namespace["create_document"], screening=screening_on, + ) + + +@pytest.mark.parametrize("scope", SCOPES) +def test_generated_documents_get_no_marker_while_uploads_and_file_sync_still_do(intake, scope): + uploaded = intake.create(scope, "uploaded", file_name="upload.md") + chat = intake.create(scope, "chat", file_name="chat.md", origin=provenance.chat_upload_origin( + conversation_id="conversation-1", message_id="conversation-1_file_1", + )) + synced = intake.create( + scope, "synced", file_name="synced.md", + xsd_logical_path="folder/synced.md", xsd_family_namespace="file-sync:source-1", + ) + for screened in (uploaded, chat, synced): + assert screened[SCREENING_FIELD]["state"] == "pending_scan" + assert screened["status"] == "Content screening pending" + assert "screening_exemption" not in screened + assert not document_is_available(screened) + generated = intake.create(scope, "generated", file_name="generated.md", screening_exemption=generated_exemption()) + assert SCREENING_FIELD not in generated + assert generated["screening_exemption"] == GENERATED + assert generated["status"] == "Queued for processing" + assert document_is_available(generated) + + +def test_the_exemption_belongs_to_one_version_and_is_never_carried_forward(intake): + generated = intake.create("personal", "doc-1", screening_exemption=generated_exemption()) + uploaded = intake.create("personal", "doc-2") + regenerated = intake.create("personal", "doc-3", screening_exemption=generated_exemption()) + assert generated["version"] == 1 and SCREENING_FIELD not in generated + assert uploaded["version"] == 2 and SCREENING_FIELD in uploaded and "screening_exemption" not in uploaded + assert regenerated["version"] == 3 and SCREENING_FIELD not in regenerated + assert regenerated["screening_exemption"] == GENERATED + + +@pytest.mark.parametrize("exemption", [ + {"reason": "generated", "version": 2}, {"reason": "upload", "version": 1}, {"reason": "generated"}, "generated", +]) +def test_a_malformed_exemption_is_refused_before_anything_is_written(intake, exemption): + with pytest.raises(ScreeningValidationError): + intake.create("personal", "doc-1", screening_exemption=exemption) + assert intake.stores["personal"].records == {} + + +def test_an_existing_marker_still_wins_over_the_exemption(screening_on): + marked = { + "id": "doc-1", "user_id": "user-1", "version": 1, "screening_exemption": GENERATED, + SCREENING_FIELD: {"state": "pending_review", "scan_id": "scan-1"}, + } + assert service.document_requires_screening(marked, screening_on.settings) is True + unmarked = {key: value for key, value in marked.items() if key != SCREENING_FIELD} + assert service.document_requires_screening(unmarked, screening_on.settings) is False + assert service.initial_document_marker(unmarked) is None + + +def test_prepare_document_upload_skips_a_generated_document(intake, monkeypatch, tmp_path): + generated = intake.create("personal", "generated", screening_exemption=generated_exemption()) + monkeypatch.setattr(service, "_document_for_upload", lambda *args, **kwargs: deepcopy(generated)) + monkeypatch.setattr(service, "begin_scan", lambda *args, **kwargs: pytest.fail("Generated content is never scanned.")) + source = tmp_path / "report.md" + source.write_text("Generated report with PRIVATE_CANARY", encoding="utf-8") + assert service.prepare_document_upload("generated", "user-1", str(source), "report.md") is None + assert intake.screening.repository.query("scan")["items"] == [] + + +@pytest.mark.parametrize("scope", ["personal", "group"]) +def test_agent_generated_uploads_create_unscreened_documents(intake, tmp_path, scope): + queued, processed = [], [] + + def write_temp(content, suffix): + path = tmp_path / f"{uuid.uuid4().hex}{suffix}" + path.write_bytes(content) + return str(path) + + namespace = { + "Any": Any, "Dict": Dict, "os": os, "uuid": uuid, + "require_generated_file_publication_allowed": lambda: None, + "_write_temp_generated_file": write_temp, + "_resolve_group_upload_target_for_current_user": lambda user_id, group_id="", default_group_id="": "group-1", + "create_document": intake.create_document, + "update_document": lambda **kwargs: None, + "process_document_upload_background": lambda **kwargs: processed.append(kwargs), + "_queue_document_upload_background_task": lambda **kwargs: queued.append(kwargs), + "invalidate_group_search_cache": lambda *args, **kwargs: None, + "invalidate_personal_search_cache": lambda *args, **kwargs: None, + "log_document_upload": lambda **kwargs: None, + "generated_screening_exemption": getattr(contracts, "generated_screening_exemption", lambda: None), + } + execute_functions("functions_simplechat_operations.py", {"_upload_generated_document_for_current_user"}, namespace) + result = namespace["_upload_generated_document_for_current_user"]( + "user-1", "summary.md", b"# Generated summary", scope, group_id="group-1" if scope == "group" else "", + ) + stored = intake.stores[scope].records[result["document"]["id"]] + assert SCREENING_FIELD not in stored + assert stored["screening_exemption"] == GENERATED + assert len(queued) == 1 and processed == [] + + +def test_upload_dispatch_processes_a_generated_document_without_screening(intake, monkeypatch): + generated = intake.create("group", "generated", screening_exemption=generated_exemption()) + calls = [] + namespace = { + "partial": __import__("functools").partial, + "get_document_metadata": lambda *args, **kwargs: deepcopy(generated), + "get_settings": lambda: intake.screening.settings, + "document_requires_screening": service.document_requires_screening, + "process_screened_upload": lambda *args, **kwargs: pytest.fail("Generated content is never screened."), + "_process_document_upload_background_impl": lambda *args, **kwargs: calls.append((args, kwargs)), + "PUBLICATION_BINDING": readiness.PUBLICATION_BINDING, + "begin_publication_processing": lambda *args, **kwargs: pytest.fail("No publication binding."), + "log_event": lambda *args, **kwargs: None, + } + execute_functions("functions_documents.py", {"process_document_upload_background"}, namespace) + namespace["process_document_upload_background"]( + "generated", "user-1", "unused-temp-path", "report.md", group_id="group-1", + ) + assert len(calls) == 1 and calls[0][1]["group_id"] == "group-1" + + +class CreationCalls(ast.NodeVisitor): + """Collect create_document calls with their innermost enclosing function.""" + + def __init__(self, module): + self.module, self.stack, self.calls = module, [], [] + + def visit_FunctionDef(self, node): + self.stack.append(node.name) + self.generic_visit(node) + self.stack.pop() + + visit_AsyncFunctionDef = visit_FunctionDef + + def visit_Call(self, node): + if isinstance(node.func, ast.Name) and node.func.id == "create_document": + exempt = any(keyword.arg == "screening_exemption" for keyword in node.keywords) + self.calls.append((self.module, self.stack[-1] if self.stack else "", exempt)) + self.generic_visit(node) + + +def creation_calls(): + found = [] + for path in sorted(APP_ROOT.glob("*.py")): + visitor = CreationCalls(path.name) + visitor.visit(ast.parse(path.read_text(encoding="utf-8-sig"), filename=path.name)) + found.extend(visitor.calls) + return found + + +def test_only_generated_creation_sites_mark_their_documents(): + calls = creation_calls() + exempt = {(module, function) for module, function, marked in calls if marked} + screened = {(module, function) for module, function, marked in calls if not marked} + assert exempt == { + ("functions_simplechat_operations.py", "_upload_generated_document_for_current_user"), + ("functions_artifact_publication.py", "_publish_generated_chat_artifact_for_user"), + } + assert screened >= { + ("route_backend_documents.py", "api_user_upload_document"), + ("route_backend_group_documents.py", "api_upload_group_document"), + ("route_backend_public_documents.py", "api_upload_public_document"), + ("route_external_public_documents.py", "external_upload_public_document"), + ("functions_group_document_management.py", "upload_group_documents"), + ("functions_public_document_management.py", "upload_public_documents"), + ("functions_documents.py", "queue_personal_workspace_upload_from_temp_file"), + ("functions_documents.py", "queue_group_workspace_upload_from_temp_file"), + ("functions_file_sync.py", "_create_document_from_remote_file"), + } + assert not exempt & screened + + +# --- Bulk scans never enroll generated documents --------------------------------------- + + +def test_scan_jobs_skip_generated_documents_and_still_scan_uploads(service_runtime): + repository = service_runtime.repository + add_document(repository, "generated", file_name="generated.md", screening_exemption=generated_exemption()) + add_document(repository, "uploaded", file_name="uploaded.txt") + job = create_job(repository) + for _attempt in range(10): + job = jobs.run_scan_job(job["id"], repository=repository) + if job["status"] in jobs.FINISHED_JOB_STATUSES: + break + items = { + item["subject"]["document_id"]: item + for item in repository.query("work_item", filters={"job_id": job["id"]})["items"] + } + assert items["generated"]["status"] == "skipped" + assert items["generated"]["error_code"] == "screening_not_required" + generated = repository.read_document(Subject("personal", "owner", "generated", "1")) + assert SCREENING_FIELD not in generated and document_is_available(generated) + assert items["uploaded"]["status"] == "completed" + uploaded = repository.read_document(Subject("personal", "owner", "uploaded", "1")) + assert uploaded[SCREENING_FIELD]["state"] == "cleared" + + +def test_a_generated_document_cannot_begin_a_scan(service_runtime): + repository = service_runtime.repository + add_document(repository, "generated", file_name="generated.md", screening_exemption=generated_exemption()) + with pytest.raises(ScreeningConflictError) as raised: + service.begin_scan(Subject("personal", "owner", "generated", "1"), "owner", repository=repository) + assert raised.value.code == "screening_not_required" + assert SCREENING_FIELD not in repository.read_document(Subject("personal", "owner", "generated", "1")) + assert repository.query("scan")["items"] == [] + + +# --- Metadata: edits and generated metadata keep a release -------------------------------- + + +class ScanStore: + def __init__(self, scan): + self.scan = scan + + def read_item(self, item, partition_key): + if item != self.scan["id"] or partition_key != self.scan["id"]: + raise LookupError("The scan record was not found.") + return deepcopy(self.scan) + + +def released_document(): + policy = {"enabled": True, "rules": [dict(ACTIVE_POLICY_RULE)]} + fingerprint = content_fingerprint([contracts.ContentUnit("unit-1", "Released text", {})]) + active_blob = {"container": "documents", "path": "doc-1/released.txt", "etag": "etag-1", "content_hash": "hash-1"} + document = { + "id": "doc-1", "user_id": "user-1", "version": 1, "file_name": "released.txt", "title": "Original", + SCREENING_FIELD: { + "state": "cleared", "scan_id": "scan-1", "source_revision": "1", "review_required": False, + "content_fingerprint": fingerprint, "policy_fingerprint": hash_payload(policy), + "canonical_ref": {"name": "units"}, "active_blob": active_blob, + }, + } + scan = { + "id": "scan-1", "kind": "scan", "subject": subject_from_document(document).to_dict(), "state": "cleared", + "coverage_complete": True, "result_status": "pass", "content_fingerprint": fingerprint, + "policy": policy, "policy_fingerprint": hash_payload(policy), "units_ref": {"name": "units"}, + "publication": { + "active_blob": active_blob, "content_fingerprint": fingerprint, + "metadata_fingerprint": contracts.metadata_fingerprint(document), + }, + } + return document, ScanStore(scan) + + +def test_editing_metadata_after_release_keeps_the_release_proof(): + document, scans = released_document() + access._require_release_proof(document, scans) + edited = {**document, "title": "Edited", "abstract": "Edited abstract", "tags": ["edited"]} + access._require_release_proof(edited, scans) + restored = {**edited, SCREENING_FIELD: {**edited[SCREENING_FIELD], "scan_id": "scan-2"}} + with pytest.raises(DocumentHeldError): + access._require_release_proof(restored, scans) + + +def test_a_rename_may_not_change_the_inspected_source_format(): + document, _scans = released_document() + assert service.validate_screened_metadata_update(document, {"file_name": "renamed.TXT", "title": "New"}) is None + with pytest.raises(ScreeningValidationError): + service.validate_screened_metadata_update(document, {"file_name": "renamed.pdf"}) + assert service.validate_screened_metadata_update( + {key: value for key, value in document.items() if key != SCREENING_FIELD}, {"file_name": "renamed.pdf"}, + ) is None + + +def test_model_generated_metadata_applies_without_a_hold(management): + env = management + env.settings["enable_content_screening"] = True + source = env.source.records["document-a"] + env.seed_release(source) + marker = deepcopy(source[SCREENING_FIELD]) + namespace = env.document_helpers + namespace["extract_document_metadata"] = lambda **kwargs: { + "title": "Generated title", "authors": ["Model author"], "abstract": "Generated abstract", + "keywords": ["generated"], "publication_date": "2026", "organization": "Contoso", + } + execute_functions("functions_documents.py", {"process_metadata_extraction_background"}, namespace) + namespace["process_metadata_extraction_background"]("document-a", "owner", group_id="group-a") + current = env.source.records["document-a"] + assert current["title"] == "Generated title" and current["abstract"] == "Generated abstract" + assert current[SCREENING_FIELD] == marker + assert current["status"] == "Metadata extraction complete" + assert document_is_available(current) + available = env.access.assert_document_available("document-a", user_id="owner", group_id="group-a") + assert available["title"] == "Generated title" + assert {write.get("title") for write in env.chunk_writes} == {"Generated title"} + assert env.blobs.metadata_writes == [] + assert env.queue.jobs == [] + + +def test_a_rename_through_update_document_keeps_the_screened_source_format(management): + env = management + env.seed_release(env.source.records["document-a"]) + original = env.source.records["document-a"]["file_name"] + suffix = Path(original).suffix + with pytest.raises(ScreeningValidationError): + env.document_helpers["update_document"]( + document_id="document-a", user_id="owner", group_id="group-a", file_name="renamed.unsupported", + ) + assert env.source.records["document-a"]["file_name"] == original + env.document_helpers["update_document"]( + document_id="document-a", user_id="owner", group_id="group-a", file_name=f"renamed{suffix}", + ) + assert env.source.records["document-a"]["file_name"] == f"renamed{suffix}" + assert document_is_available(env.source.records["document-a"]) + + +# --- Publication: no screening reservation ----------------------------------------------- + + +@pytest.fixture +def screened_group_publication(publication, tmp_path): + """The real publication engine and create_document with screening on and an active policy.""" + import importlib.util + + from content_screening import repository as screening_repository + from content_screening.storage import ScreeningStorage + from test_content_screening_persistence import FakeBlobService + + env = publication + patch = env.scoped_monkeypatch + env.settings["enable_content_screening"] = True + metadata = FakeCosmos() + repository = ScreeningRepository(metadata, {"group": env.source}) + patch.setattr(env.config, "cosmos_content_screening_container", metadata) + patch.setattr(screening_repository, "get_repository", lambda: repository) + policy = default_policy() + policy.update(enabled=True, rules=[dict(ACTIVE_POLICY_RULE)]) + repository.save_policy("global", "global", policy, "administrator") + spec = importlib.util.spec_from_file_location( + "content_screening.service", APP_ROOT / "content_screening" / "service.py", + ) + screening = importlib.util.module_from_spec(spec) + patch.setitem(sys.modules, "content_screening.service", screening) + spec.loader.exec_module(screening) + storage = ScreeningStorage(FakeBlobService()) + patch.setattr(screening, "_storage", lambda value=None: value if value is not None else storage) + patch.setitem(env.document_helpers, "initial_document_marker", screening.initial_document_marker) + patch.setattr( + sys.modules["functions_documents"], "get_document_metadata", + env.document_helpers["get_document_metadata"], raising=False, + ) + + def create_destination(**values): + env.publication_calls["create"].append(deepcopy(values)) + return env.document_helpers["create_document"](**values) + + patch.setattr(env.canonical, "create_document", create_destination) + source_path = tmp_path / "accepted.md" + source_path.write_bytes(env.content) + prepared = [] + + def prepare(): + prepared.append(screening.prepare_document_upload( + env.target, REQUESTER, str(source_path), env.source.records[env.target]["file_name"], + group_id="group-a", + )) + + env.publication_state["queue_hook"] = prepare + env.screening_repository = repository + env.prepared = prepared + return env + + +def test_published_destination_is_unscreened_and_approval_needs_no_reservation(screened_group_publication): + env = screened_group_publication + created = submit(env) + assert SCREENING_FIELD not in created + assert created["screening_exemption"] == GENERATED + assert env.publication_calls["create"][0]["screening_exemption"] == GENERATED + assert not any(str(key).startswith("screening_") for key in receipt(env)) + view = sharing(env) + assert "approve_artifact" in view.get_json()["actions"] + assert "screening_exemption" not in str(view.get_json()) + response = decide(env, "approve") + assert_receipt(response, env, "approve", "queued", "approved", 202) + current = env.source.read_item(env.target, env.target) + saved = receipt(env) + assert SCREENING_FIELD not in current + assert saved["decision"]["choice"] == "approved" and "operation_id" not in saved["decision"] + assert not any(str(key).startswith("screening_") for key in saved) + assert len(env.publication_calls["queue_attempts"]) == 1 + assert env.prepared == [None] + assert env.screening_repository.query("scan")["items"] == [] + detail = env.client.get(f"/api/group_documents/{env.target}?group_id=group-a") + assert "screening_exemption" not in str(detail.get_json()) + + +def test_publication_readiness_counts_a_marker_free_destination_as_screened(): + binding = { + "version": 1, "receipt_id": "receipt-1", "document_version": 1, "content_sha256": "sha-1", + "conversation_id": "conversation-1", "artifact_message_id": "artifact-1", + } + saved = { + "id": "receipt-1", "content_sha256": "sha-1", "document_version": 1, "actor_user_id": "user-1", + "destination": {"workspace_scope": "group", "group_id": "group-1"}, + "artifact_reference": {"conversation_id": "conversation-1", "artifact_message_id": "artifact-1"}, + } + destination = { + "id": "doc-1", "group_id": "group-1", "version": 1, "user_id": "user-1", + "generated_artifact_publication_receipt_id": "receipt-1", "screening_exemption": GENERATED, + readiness.PUBLICATION_BINDING: binding, + readiness.PUBLICATION_PROCESSING: {"binding": binding, "state": "complete", "indexed_chunks": 2}, + } + ready = readiness.inspect_publication_readiness( + saved, destination, available_reader=lambda *args, **kwargs: deepcopy(destination), + index_count=lambda *args: 2, + ) + assert ready == {"processing": "complete", "screening": "not_required", "index": "ready"} + + def held(*args, **kwargs): + raise DocumentHeldError() + + denied = readiness.inspect_publication_readiness(saved, destination, available_reader=held, index_count=lambda *args: 2) + assert denied["reason_code"] == "publication_screening_unavailable" + + +def test_the_publication_reservation_and_metadata_rescan_are_gone(): + removed = { + "consume_artifact_publication_screening_scan", "_consume_publication_reservation", + "publication_screening_reservation", "_enroll_screening_reservation", + "_reservation_has_screening_history", "_bootstrap_retry_operation_id", "queue_metadata_rescan", + } + defined = set() + for path in [*APP_ROOT.glob("*.py"), *(APP_ROOT / "content_screening").glob("*.py")]: + source = path.read_text(encoding="utf-8-sig") + tree = ast.parse(source, filename=path.name) + defined |= { + node.name for node in ast.walk(tree) + if isinstance(node, (ast.FunctionDef, ast.AsyncFunctionDef)) and node.name in removed + } + assert "screening_reservation" not in source, path.name + assert defined == set() + + +@pytest.mark.parametrize("first", ["content_screening.service", "functions_artifact_publication_readiness"]) +def test_screening_and_readiness_modules_keep_real_cold_imports_below_azure_bootstrap(first): + second = ( + "functions_artifact_publication_readiness" + if first == "content_screening.service" else "content_screening.service" + ) + code = ( + "import importlib, socket, sys\n" + "def blocked(*args, **kwargs):\n" + " raise RuntimeError('Network forbidden during cold import')\n" + "socket.create_connection = blocked\n" + "socket.socket.connect = blocked\n" + f"importlib.import_module({first!r})\n" + f"importlib.import_module({second!r})\n" + "if any(name in sys.modules for name in ('config', 'functions_settings', 'functions_artifact_publication')):\n" + " raise RuntimeError('Readiness or screening initialized application owners')\n" + ) + result = subprocess.run( + [sys.executable, "-c", code], cwd=APP_ROOT, capture_output=True, text=True, timeout=60, + ) + assert result.returncode == 0, result.stderr + + +if __name__ == "__main__": + sys.exit(pytest.main([str(Path(__file__).resolve()), *sys.argv[1:]])) diff --git a/functional_tests/test_group_document_management.py b/functional_tests/test_group_document_management.py index f16404329..d8e51f4b5 100644 --- a/functional_tests/test_group_document_management.py +++ b/functional_tests/test_group_document_management.py @@ -1,11 +1,12 @@ # test_group_document_management.py """ Functional tests for immutable-target group document management. -Version: 0.261.194 +Version: 0.261.230 Implemented in: 0.261.129 A tag vocabulary conflict answers one coded sentence, from the pre-check or a lost patch: 0.261.167 New tags are defined before any document carries them, so a conflict writes no document: 0.261.168 The real document definitions receive the server-only document provenance helpers they import: 0.261.194 +Screened metadata edits apply directly and never rewrite the pinned release blob: 0.261.230 Real Flask routes, management/access/policy modules, conditional document writes, revision deletion and canonical downloads run against isolated storage, queues, @@ -250,15 +251,6 @@ def management(environment): def chunk_update(**kwargs): env.chunk_writes.append(deepcopy(kwargs)) - def metadata_rescan(document_item, updates, actor_id): - current = env.source.read_item(document_item["id"], document_item["id"]) - changed = {**current, **updates, SCREENING_FIELD: { - **current[SCREENING_FIELD], "state": "pending_scan", "scan_id": f"rescan-{current['id']}", - }} - return env.source.replace_item( - current["id"], changed, etag=document_item["_etag"], match_condition="match", - ) - def prepare_delete(document_item, actor_id): marker = {**document_item[SCREENING_FIELD], "state": "deleting"} env.source.change(document_item["id"], content_screening=marker) @@ -267,9 +259,9 @@ def prepare_delete(document_item, actor_id): # The application modules are loaded only after the base fixture's network # and bootstrap seams are active; real shared mutation definitions run below. from content_screening.contracts import ( - CONTENT_METADATA_FIELDS, SCREENING_FIELD, DocumentHeldError, ScreeningConflictError, - ScreeningError, ScreeningValidationError, document_is_available, require_document_available, - subject_from_document, + SCREENING_EXEMPTION_FIELD, SCREENING_FIELD, DocumentHeldError, ScreeningConflictError, + ScreeningError, ScreeningValidationError, document_is_available, is_generated_screening_exempt, + require_document_available, subject_from_document, ) namespace = env.document_helpers @@ -286,11 +278,11 @@ def prepare_delete(document_item, actor_id): "ScreeningError": ScreeningError, "ScreeningValidationError": ScreeningValidationError, "DocumentHeldError": DocumentHeldError, "document_is_available": document_is_available, "require_document_available": require_document_available, "subject_from_document": subject_from_document, - "CONTENT_METADATA_FIELDS": CONTENT_METADATA_FIELDS, + "SCREENING_EXEMPTION_FIELD": SCREENING_EXEMPTION_FIELD, + "is_generated_screening_exempt": is_generated_screening_exempt, "Path": Path, "public_document_payload": env.access.public_document_payload, "read_available_document_bytes": env.access.read_available_document_bytes, "current_extraction": lambda _document_id: None, "is_publication": lambda *_args: False, - "queue_metadata_rescan": metadata_rescan, "add_file_task_to_file_processing_log": Mock(), "calculate_processing_percentage": lambda item: item.get("percentage_complete", 0), "get_all_chunks": lambda document_id, user_id, group_id=None, public_workspace_id=None: [{"id": f"{document_id}-chunk"}], @@ -314,6 +306,8 @@ def prepare_delete(document_item, actor_id): "ORIGIN_FIELD_NAMES", "ORIGIN_KIND_FIELD", "DocumentOriginError", "apply_document_provenance", "remember_document_origin_summary", "validate_origin", )}) + # Metadata edits apply directly; only the real source-format rule guards a screened rename. + execute_functions("content_screening/service.py", {"validate_screened_metadata_update"}, namespace) execute_functions("functions_documents.py", { "_get_blob_service_client", "_blob_exists", "_get_documents_container", "_upsert_document_and_sync_access_index", "update_document", "propagate_tags_to_blob_metadata", @@ -530,17 +524,36 @@ def test_required_projection_failure_never_returns_an_updated_receipt(management assert env.source.records["document-a"]["tags"] == ["changed"] -def test_metadata_rescan_has_a_bound_queued_receipt(management): +def test_screened_metadata_edit_applies_directly_and_keeps_the_release(management): env = management env.settings["enable_content_screening"] = True env.seed_release(env.source.records["document-a"]) + marker = deepcopy(env.source.records["document-a"]["content_screening"]) response = env.client.patch(f"{ROOT}/document-a", json={"abstract": "Updated abstract"}) - assert response.status_code == 202, response.get_json() - assert response.get_json()["status"] == "queued" + assert response.status_code == 200, response.get_json() + assert response.get_json()["status"] == "updated" assert response.get_json()["document_id"] == "document-a" assert response.get_json()["group_id"] == "group-a" assert response.get_json()["updated_fields"] == ["abstract"] - assert env.source.records["document-a"]["content_screening"]["state"] == "pending_scan" + current = env.source.records["document-a"] + assert current["abstract"] == "Updated abstract" + assert current["content_screening"] == marker + assert env.queue.jobs == [] + + +def test_screened_tag_edit_updates_chunks_but_never_the_pinned_release_blob(management): + env = management + env.settings["enable_content_screening"] = True + env.seed_release(env.source.records["document-a"]) + marker = deepcopy(env.source.records["document-a"]["content_screening"]) + response = env.client.patch(f"{ROOT}/document-a", json={"tags": ["reviewed"]}) + assert response.status_code == 200, response.get_json() + assert response.get_json()["status"] == "updated" + current = env.source.records["document-a"] + assert current["tags"] == ["reviewed"] + assert current["content_screening"] == marker + assert [write.get("document_tags") for write in env.chunk_writes] == [["reviewed"]] + assert env.blobs.metadata_writes == [] def test_incoming_download_uses_source_bytes_with_same_name_in_recipient(management): diff --git a/functional_tests/test_group_document_publication_screening_bootstrap.py b/functional_tests/test_group_document_publication_screening_bootstrap.py deleted file mode 100644 index d4f52affe..000000000 --- a/functional_tests/test_group_document_publication_screening_bootstrap.py +++ /dev/null @@ -1,734 +0,0 @@ -# test_group_document_publication_screening_bootstrap.py -""" -Functional tests for receipt-bound screening reservation consumption. -Version: 0.261.131 -Implemented in: 0.261.131 - -Real scoped Flask decisions, canonical publication, document creation, screening -admission, conditional repositories and durable scan queues use isolated stores. -The shared publication fixture blocks network access. No models or Azure services -are called; fake storage is not evidence of live Cosmos consistency. -""" - -from copy import deepcopy -import importlib.util -from pathlib import Path -import subprocess -import sys - -import pytest - -from test_content_screening_persistence import FakeBlobService, FakeCosmos -from test_group_document_management import StoreFailure, management # noqa: F401 -from test_group_document_publication import ( - BINDING, OPERATION, RECEIPTS, REQUESTER, ROOT, after_stage_claim, assert_receipt, - change_receipt, decide, publication, publication_modules, receipt, set_actor, sharing, submit, -) -from test_group_document_read_apis import environment # noqa: F401 -from test_support.agent_delegation import APP_ROOT - -from content_screening import repository as screening_repository -from content_screening.contracts import ContentUnit, DocumentHeldError, subject_from_document -from content_screening.policies import default_policy -from content_screening.repository import ScreeningRepository -from content_screening.storage import ScreeningStorage -from functions_artifact_publication_readiness import ( - PUBLICATION_SCREENING_CONSUMPTION as CONSUMPTION, - PUBLICATION_SCREENING_RESERVATION as RESERVATION, - publication_screening_reservation, -) - - -class InjectedCrash(BaseException): - """Model process exit without letting request-level exception handlers run.""" - - -@pytest.fixture -def screened_publication(publication, tmp_path): - env = publication - patch = env.scoped_monkeypatch - env.settings["enable_content_screening"] = True - metadata = FakeCosmos() - repository = ScreeningRepository(metadata, {"group": env.source}) - patch.setattr(env.config, "cosmos_content_screening_container", metadata) - patch.setattr(screening_repository, "get_repository", lambda: repository) - policy = default_policy() - policy.update(enabled=True, rules=[{ - "id": "restricted", "name": "Restricted", "type": "literal", "enabled": True, - "severity": "high", "category": "sensitive", "values": ["PRIVATE_CANARY"], - }]) - repository.save_policy("global", "global", policy, "administrator") - spec = importlib.util.spec_from_file_location( - "content_screening.service", APP_ROOT / "content_screening" / "service.py", - ) - screening = importlib.util.module_from_spec(spec) - patch.setitem(sys.modules, "content_screening.service", screening) - spec.loader.exec_module(screening) - storage = ScreeningStorage(FakeBlobService()) - patch.setattr(screening, "_storage", lambda value=None: value if value is not None else storage) - patch.setitem(env.document_helpers, "initial_document_marker", screening.initial_document_marker) - patch.setitem( - env.document_helpers, "prepare_document_deletion", - lambda item, actor: screening.prepare_document_deletion(item, actor, repository=repository, storage=storage), - ) - patch.setattr( - sys.modules["functions_documents"], "get_document_metadata", - env.document_helpers["get_document_metadata"], raising=False, - ) - - def create_destination(**values): - env.publication_calls["create"].append(deepcopy(values)) - return env.document_helpers["create_document"](**values) - - patch.setattr(env.canonical, "create_document", create_destination) - source_path = tmp_path / "accepted.md" - source_path.write_bytes(env.content) - - def prepare(): - return screening.prepare_document_upload( - env.target, REQUESTER, str(source_path), env.source.records[env.target]["file_name"], - group_id="group-a", - ) - - env.publication_state["queue_hook"] = prepare - env.screening = screening - env.screening_repository = repository - env.screening_metadata = metadata - env.screening_storage = storage - env.prepare_screened_upload = prepare - return env - - -def current_document(env): - return env.source.read_item(env.target, env.target) - - -def scan_records(env, kind="scan"): - page = env.screening_repository.query( - kind, "group:group-a", filters={"document_id": env.target}, page_size=100, - ) - return page["items"] - - -def crash_before_queue(env): - original = env.canonical._stage - - def stage(artifact, saved, name, *, complete=False): - if name == "approval_queue" and not complete: - raise InjectedCrash() - return original(artifact, saved, name, complete=complete) - - with env.scoped_monkeypatch.context() as patch: - patch.setattr(env.canonical, "_stage", stage) - with pytest.raises(InjectedCrash): - decide(env, "approve") - - -def assert_bootstrap_denied(env): - view = sharing(env) - response = decide(env, "approve") - saved = receipt(env) - assert response.status_code == 409, response.get_json() - assert "Cancel" in response.get_json()["message"] - assert "again" in response.get_json()["message"] - assert "approve_artifact" not in view.get_json()["actions"] - assert "approve_artifact" not in view.get_json()["publication"]["actions"] - assert not env.publication_calls["queue_attempts"] - assert "decision" not in saved - return response - - -def test_reserved_scan_uuid_bootstraps_once_without_granting_content_or_sharing(screened_publication): - env = screened_publication - initial = submit(env) - initial_receipt = receipt(env) - original_proof = initial_receipt[RESERVATION] - stored_proof = publication_screening_reservation(initial) - assert initial["content_screening"]["scan_id"] - assert original_proof == stored_proof - assert CONSUMPTION not in initial_receipt - pending = sharing(env) - assert "approve_artifact" in pending.get_json()["actions"] - response = decide(env, "approve") - assert_receipt(response, env, "approve", "queued", "approved", 202) - current = current_document(env) - saved = receipt(env) - scans = scan_records(env) - jobs = list(env.screening_metadata.query_items( - "SELECT * FROM c WHERE c.kind = @kind", parameters=[{"name": "@kind", "value": "job"}], - )) - assert len(env.publication_calls["queue_attempts"]) == len(scans) == len(jobs) == 1 - assert saved[RESERVATION] == original_proof - assert saved[CONSUMPTION] == { - "operation_id": current[OPERATION]["id"], "fingerprint": original_proof["fingerprint"], - "scan_id": initial["content_screening"]["scan_id"], - } - assert current["content_screening"]["state"] == "pending_scan" - assert current["content_screening"]["generation"] == 2 - assert current["content_screening"]["scan_id"] == initial["content_screening"]["scan_id"] - with pytest.raises(DocumentHeldError): - env.access.assert_document_available(env.target, user_id="manager", group_id="group-a") - with pytest.raises(DocumentHeldError): - env.access.read_available_document_bytes(env.target, user_id="manager", group_id="group-a") - download = env.client.get(f"{ROOT}/{env.target}/download") - share = env.client.post( - f"{ROOT}/{env.target}/share", - json={"expected_etag": current["_etag"], "target_group_id": "group-b"}, - ) - detail = env.client.get(f"/api/group_documents/{env.target}?group_id=group-a") - repeated = decide(env, "approve") - assert download.status_code == share.status_code == repeated.status_code == 409 - assert detail.status_code == 200 - assert detail.get_json()["content_screening"]["available"] is False - assert detail.get_json()["document_actions"] == [] - assert "share" not in detail.get_json()["document_collaboration_actions"] - serialized = str(detail.get_json()) - for private in (RESERVATION, CONSUMPTION, BINDING, original_proof["fingerprint"], current[OPERATION]["id"]): - assert private not in serialized - assert len(env.publication_calls["queue_attempts"]) == 1 - assert not env.blobs.downloads - - -def test_enrollment_fingerprints_the_stored_destination_not_the_creation_return_value(screened_publication): - env = screened_publication - original = env.canonical.create_document - returned = {} - - def create(**values): - original(**values) - stored = env.source.read_item(values["document_id"], values["document_id"]) - returned.update(stored) - returned["content_screening"] = env.screening.initial_document_marker(stored) - return returned - - env.scoped_monkeypatch.setattr(env.canonical, "create_document", create) - initial = submit(env) - saved = receipt(env) - stored_proof = publication_screening_reservation(initial) - in_memory_proof = publication_screening_reservation(returned) - assert saved[RESERVATION] == stored_proof - assert saved[RESERVATION] != in_memory_proof - response = decide(env, "approve") - assert_receipt(response, env, "approve", "queued", "approved", 202) - - -def test_replaying_legacy_preparation_cannot_mint_missing_reservation_proof(screened_publication): - env = screened_publication - submit(env, legacy=True) - message = env.messages.read_item(env.artifact_id, "private-conversation") - metadata = deepcopy(message["metadata"]) - legacy = metadata[RECEIPTS][env.receipt_id] - legacy.pop(RESERVATION) - legacy["stages"].pop("prepare") - env.messages.change(env.artifact_id, metadata=metadata) - env.source.records[env.target].pop("generated_artifact_publication_receipt_id") - submit(env, legacy=True) - saved = receipt(env) - assert RESERVATION not in saved - assert_bootstrap_denied(env) - - -def test_scan_racing_preparation_cannot_be_hidden_before_proof_enrollment(screened_publication): - env = screened_publication - original = env.canonical.update_document - - def update(**values): - original(**values) - if "generated_artifact_publication_receipt_id" not in values: - return - env.target = values["document_id"] - env.receipt_id = values["generated_artifact_publication_receipt_id"] - current = current_document(env) - scan = env.screening.begin_scan( - subject_from_document(current), REQUESTER, repository=env.screening_repository, - scan_id=current["content_screening"]["scan_id"], - ) - env.source.change(env.target, content_screening=current["content_screening"]) - env.screening_metadata.documents.pop((scan["id"], scan["id"])) - - env.scoped_monkeypatch.setattr(env.canonical, "update_document", update) - with pytest.raises(ValueError, match="Cancel"): - submit(env) - saved = receipt(env) - assert RESERVATION not in saved - assert saved[CONSUMPTION]["scan_id"] - assert_bootstrap_denied(env) - - -@pytest.mark.parametrize("state", ["pending_scan", "pending_review", "scanning", "scan_error", "incomplete", "publishing"]) -def test_actual_scan_attempt_is_never_bootstrap_admission(screened_publication, state): - env = screened_publication - initial = submit(env) - scan = env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - env.screening._save_scan(env.screening_repository, scan, state=state) - env.screening._set_document_state(env.screening_repository, scan, state) - held = deepcopy(current_document(env)["content_screening"]) - assert_bootstrap_denied(env) - current = current_document(env) - assert current["content_screening"] == held - - -@pytest.mark.parametrize("remove_row", [False, True]) -def test_failed_scan_marker_reset_and_removed_scan_cannot_unconsume_receipt(screened_publication, remove_row): - env = screened_publication - initial = submit(env) - scan = env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - env.screening._record_failure(env.screening_repository, scan["id"], "fixture_failure") - interrupted_receipt = receipt(env) - consumed = interrupted_receipt[CONSUMPTION] - env.source.change(env.target, content_screening=initial["content_screening"], status=initial["status"]) - if remove_row: - env.screening_metadata.documents.pop((scan["id"], scan["id"])) - assert_bootstrap_denied(env) - saved = receipt(env) - assert saved[CONSUMPTION] == consumed - - -@pytest.mark.parametrize("failure", ["before_scan_create", "after_scan_create"]) -def test_scan_consumption_is_written_before_scan_creation_or_marker_replacement(screened_publication, failure): - env = screened_publication - initial = submit(env) - original_create = env.screening_repository.create - - def create(record): - saved = receipt(env) - assert saved[CONSUMPTION]["scan_id"] == record["id"] - if failure == "after_scan_create": - original_create(record) - raise StoreFailure() - - with env.scoped_monkeypatch.context() as patch: - patch.setattr(env.screening_repository, "create", create) - with pytest.raises(StoreFailure): - env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - current = current_document(env) - assert current["content_screening"] == initial["content_screening"] - env.screening_metadata.documents.pop( - (initial["content_screening"]["scan_id"], initial["content_screening"]["scan_id"]), None, - ) - assert_bootstrap_denied(env) - - -@pytest.mark.parametrize("enrolled", [False, True]) -def test_absent_marker_uses_current_destination_policy_and_never_bootstraps(screened_publication, enrolled): - env = screened_publication - if not enrolled: - env.settings["enable_content_screening"] = False - submit(env) - env.settings["enable_content_screening"] = True - env.source.records[env.target].pop("content_screening", None) - assert_bootstrap_denied(env) - - -def test_disabling_screening_does_not_turn_a_persisted_reservation_into_available_content(screened_publication): - env = screened_publication - submit(env) - env.settings["enable_content_screening"] = False - assert_bootstrap_denied(env) - - -def test_enabled_empty_policy_preserves_ordinary_unscreened_approval(screened_publication): - env = screened_publication - baseline = env.screening_repository.get_policy("global", "global") - env.screening_repository.save_policy( - "global", "global", {**default_policy(), "enabled": True}, "administrator", etag=baseline["_etag"], - ) - initial = submit(env) - response = decide(env, "approve") - saved = receipt(env) - assert "content_screening" not in initial - assert RESERVATION not in saved and CONSUMPTION not in saved - assert_receipt(response, env, "approve", "queued", "approved", 202) - assert len(env.publication_calls["queue_attempts"]) == 1 - - -@pytest.mark.parametrize("failure", ["process_exit", "consumption_acknowledgement"]) -def test_same_operation_prequeue_crash_recovers_exactly_once(screened_publication, failure): - env = screened_publication - initial = submit(env) - if failure == "process_exit": - crash_before_queue(env) - else: - original = env.messages.replace_item - failed = False - - def replace(item, body, **kwargs): - nonlocal failed - result = original(item, body, **kwargs) - candidate = body.get("metadata", {}).get(RECEIPTS, {}).get(env.receipt_id, {}) - if CONSUMPTION in candidate and not failed: - failed = True - raise TimeoutError("Private acknowledgement canary") - return result - - with env.scoped_monkeypatch.context() as patch: - patch.setattr(env.messages, "replace_item", replace) - failed_response = decide(env, "approve") - assert_receipt(failed_response, env, "approve", "partial", "approval_failed", 207) - interrupted = current_document(env) - interrupted_receipt = receipt(env) - consumed = interrupted_receipt[CONSUMPTION] - assert "scan_id" not in consumed - assert consumed["operation_id"] == interrupted[OPERATION]["id"] - assert interrupted["content_screening"] == initial["content_screening"] - assert "approval_queue" not in interrupted_receipt["stages"] - assert not env.publication_calls["queue_attempts"] - view = sharing(env) - assert "approve_artifact" in view.get_json()["actions"] - response = decide(env, "approve", etag=view.get_json()["etag"]) - current = current_document(env) - scans = scan_records(env) - assert_receipt(response, env, "approve", "unchanged", "approved", 200) - assert current[OPERATION]["id"] == interrupted[OPERATION]["id"] - assert current[OPERATION]["execution_token"] != interrupted[OPERATION]["execution_token"] - assert len(scans) == len(env.publication_calls["queue_attempts"]) == 1 - assert current["content_screening"]["state"] == "pending_scan" - repeated = decide(env, "approve") - assert repeated.status_code == 409 - assert len(env.publication_calls["queue_attempts"]) == 1 - - -@pytest.mark.parametrize("change", ["operation_id", "actor", "missing_token"]) -def test_a_different_operation_or_actor_cannot_borrow_consumed_prequeue_retry(screened_publication, change): - env = screened_publication - submit(env) - crash_before_queue(env) - interrupted = current_document(env) - consumed = receipt(env)[CONSUMPTION] - if change == "operation_id": - env.source.change(env.target, **{OPERATION: {**interrupted[OPERATION], "id": "another-operation"}}) - elif change == "actor": - set_actor(env, "admin") - else: - env.source.change(env.target, **{OPERATION: {**interrupted[OPERATION], "execution_token": None}}) - view = sharing(env) - response = decide(env, "approve") - saved = receipt(env) - assert response.status_code == 409 - assert "Cancel" in response.get_json()["message"] - assert "approve_artifact" not in view.get_json()["actions"] - assert not env.publication_calls["queue_attempts"] - assert saved[CONSUMPTION] == consumed - with pytest.raises(ValueError, match="Cancel"): - env.canonical.decide_artifact_publication( - "manager", current_document(env), "approved", operation_id="another-operation", - ) - assert not env.publication_calls["queue_attempts"] - - -@pytest.mark.parametrize("recreation", ["new_reservation", "copied_marker_new_resource"]) -def test_recreated_destination_cannot_reuse_old_receipt_fingerprint(screened_publication, recreation): - env = screened_publication - initial = submit(env) - original_proof = receipt(env)[RESERVATION] - env.source.delete_item(env.target, env.target) - replacement = deepcopy(initial) - if recreation == "new_reservation": - replacement["content_screening"] = env.screening.initial_document_marker(replacement) - else: - replacement["_rid"] = "another-physical-cosmos-document" - env.source.create_item(replacement) - assert_bootstrap_denied(env) - saved = receipt(env) - assert saved[RESERVATION] == original_proof - - -def test_unproven_legacy_reservation_can_cancel_and_request_a_new_admissible_receipt(screened_publication): - env = screened_publication - initial = submit(env, legacy=True) - message = env.messages.read_item(env.artifact_id, "private-conversation") - metadata = deepcopy(message["metadata"]) - metadata[RECEIPTS][env.receipt_id].pop(RESERVATION) - env.messages.change(env.artifact_id, metadata=metadata) - assert_bootstrap_denied(env) - old_id, old_receipt = env.target, env.receipt_id - set_actor(env, REQUESTER) - cancelled = decide(env, "cancel") - assert_receipt(cancelled, env, "cancel", "applied", "cancelled", 200) - assert old_id not in env.source.records - fresh = submit(env, request_id="new-screening-publication-request") - assert fresh["id"] != old_id - assert fresh["content_screening"]["scan_id"] != initial["content_screening"]["scan_id"] - assert env.receipt_id != old_receipt - set_actor(env, "manager") - approved = decide(env, "approve") - assert_receipt(approved, env, "approve", "queued", "approved", 202) - receipts = env.messages.read_item(env.artifact_id, "private-conversation")["metadata"][RECEIPTS] - assert receipts[old_receipt]["decision"]["choice"] == "cancelled" - assert RESERVATION not in receipts[old_receipt] - assert len(receipts) == len(env.publication_calls["create"]) == 2 - assert len(env.publication_calls["queue_attempts"]) == 1 - - -@pytest.mark.parametrize("action", ["cancel", "reject"]) -def test_negative_cleanup_preserves_existing_consumption_forever(screened_publication, action): - env = screened_publication - initial = submit(env) - env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - consumed = receipt(env)[CONSUMPTION] - if action == "cancel": - set_actor(env, REQUESTER) - response = decide(env, action) - saved = receipt(env) - assert_receipt(response, env, action, "applied", "cancelled" if action == "cancel" else "rejected", 200) - assert env.target not in env.source.records - assert saved[CONSUMPTION] == consumed - assert not env.publication_calls["queue_attempts"] - - -@pytest.mark.parametrize("when", ["before", "after_bytes", "queue_claim"]) -def test_screening_bootstrap_never_bypasses_original_source_authority(screened_publication, when): - env = screened_publication - submit(env) - - def revoke(): - env.publication_state["source_allowed"] = False - - if when == "before": - revoke() - elif when == "after_bytes": - env.publication_state["download_hook"] = revoke - else: - after_stage_claim(env, "approval_queue", revoke) - response = decide(env, "approve") - saved = receipt(env) - assert response.status_code == (207 if when == "queue_claim" else 409) - assert not env.publication_calls["queue_attempts"] - if when == "queue_claim": - current = current_document(env) - assert saved[CONSUMPTION]["operation_id"] == current[OPERATION]["id"] - assert saved["stages"]["approval_queue"] == "started" - else: - assert CONSUMPTION not in saved and "decision" not in saved - - -def test_bootstrap_still_requires_the_exact_original_artifact_bytes(screened_publication): - env = screened_publication - submit(env) - env.content = b"Different generated artifact bytes" - response = decide(env, "approve") - saved = receipt(env) - assert response.status_code == 409 - assert CONSUMPTION not in saved and "decision" not in saved - assert not env.publication_calls["queue_attempts"] - - -@pytest.mark.parametrize("when", ["before", "after"]) -def test_scan_cannot_start_without_acknowledged_write_ahead_consumption(screened_publication, when): - env = screened_publication - initial = submit(env) - original = env.messages.replace_item - - def replace(item, body, **kwargs): - candidate = body.get("metadata", {}).get(RECEIPTS, {}).get(env.receipt_id, {}) - if CONSUMPTION not in candidate: - return original(item, body, **kwargs) - if when == "after": - original(item, body, **kwargs) - raise TimeoutError("Private receipt acknowledgement canary") - - with env.scoped_monkeypatch.context() as patch: - patch.setattr(env.messages, "replace_item", replace) - with pytest.raises(TimeoutError): - env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - current = current_document(env) - scans = scan_records(env) - saved = receipt(env) - assert not scans - assert current["content_screening"] == initial["content_screening"] - if when == "after": - assert saved[CONSUMPTION]["scan_id"] == initial["content_screening"]["scan_id"] - assert_bootstrap_denied(env) - else: - assert CONSUMPTION not in saved - response = decide(env, "approve") - assert_receipt(response, env, "approve", "queued", "approved", 202) - - -def test_concurrent_scan_consumption_wins_receipt_cas_before_approval(screened_publication): - env = screened_publication - initial = submit(env) - - def before_write(operation, item, body): - candidate = body.get("metadata", {}).get(RECEIPTS, {}).get(env.receipt_id, {}) - if operation == "replace" and candidate.get("decision", {}).get("choice") == "approved": - env.messages.before_write = None - env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - - env.messages.before_write = before_write - response = decide(env, "approve") - saved = receipt(env) - scans = scan_records(env) - assert response.status_code == 409 - assert "decision" not in saved - assert saved[CONSUMPTION]["operation_id"] == f"scan:{initial['content_screening']['scan_id']}" - assert len(scans) == 1 - assert not env.publication_calls["queue_attempts"] - - -@pytest.mark.parametrize("change", ["marker", "receipt", "scan"]) -def test_queue_admission_rereads_reservation_and_consumption_after_stage_claim(screened_publication, change): - env = screened_publication - initial = submit(env) - - def invalidate(): - if change == "marker": - env.source.change(env.target, content_screening=env.screening.initial_document_marker(initial)) - elif change == "receipt": - saved = receipt(env) - change_receipt(env, **{CONSUMPTION: {**saved[CONSUMPTION], "operation_id": "different-operation"}}) - else: - env.screening.begin_scan( - subject_from_document(initial), REQUESTER, repository=env.screening_repository, - scan_id=initial["content_screening"]["scan_id"], - ) - - after_stage_claim(env, "approval_queue", invalidate) - response = decide(env, "approve") - saved = receipt(env) - assert_receipt(response, env, "approve", "partial", "approval_failed", 207) - assert saved["stages"]["approval_queue"] == "started" - assert not env.publication_calls["queue_attempts"] - repeated = decide(env, "approve") - assert repeated.status_code == 409 - assert not env.publication_calls["queue_attempts"] - - -def test_unknown_queue_acknowledgement_never_allows_bootstrap_replay_even_after_reset(screened_publication): - env = screened_publication - initial = submit(env) - env.publication_state["queue_failure"] = True - response = decide(env, "approve") - saved = receipt(env) - assert_receipt(response, env, "approve", "partial", "approval_failed", 207) - assert saved["stages"]["approval_queue"] == "started" - assert saved[CONSUMPTION]["scan_id"] == initial["content_screening"]["scan_id"] - env.source.change(env.target, content_screening=initial["content_screening"]) - env.screening_metadata.documents.pop( - (initial["content_screening"]["scan_id"], initial["content_screening"]["scan_id"]), - ) - repeated = decide(env, "approve") - after = receipt(env) - assert repeated.status_code == 409 - assert after[CONSUMPTION] == saved[CONSUMPTION] - assert len(env.publication_calls["queue_attempts"]) == 1 - - -@pytest.mark.parametrize("change", [ - {"blob_path": "group-a/unreviewed.md"}, - {"archived_blob_path": "group-a/older.md"}, - {"num_chunks": 1}, - {"number_of_pages": 1}, - {"content": "prior content"}, - {"version": 2}, - {"revision_family_id": "older-revision"}, - {"generated_artifact_publication_processing": {"state": "running"}}, - {"generated_artifact_publication_processing": {}}, - {"screening_provenance": {}}, -]) -def test_prior_content_or_native_processing_precludes_bootstrap(screened_publication, change): - env = screened_publication - submit(env) - if "version" in change: - current = current_document(env) - change = {**change, BINDING: {**current[BINDING], "document_version": 2}} - change_receipt(env, document_version=2) - env.source.change(env.target, **change) - assert_bootstrap_denied(env) - - -@pytest.mark.parametrize("change", [ - {"generation": 2}, {"generation": True}, {"review_required": True}, {"finding_count": 1}, - {"result_ref": {"private": "result"}}, {"review_decision": {"action": "reject"}}, - {"history": []}, {"previous_marker": {}}, {"content_fingerprint": "prior-content"}, -]) -def test_scan_results_reviews_or_history_are_not_initial_reservations(screened_publication, change): - env = screened_publication - initial = submit(env) - env.source.change(env.target, content_screening={**initial["content_screening"], **change}) - assert_bootstrap_denied(env) - - -@pytest.mark.parametrize("kind", ["scan", "review", "finding", "model_window", "event", "audit", "checkpoint", "work_item"]) -def test_private_scan_or_review_history_denies_even_with_an_initial_looking_marker(screened_publication, kind): - env = screened_publication - initial = submit(env) - env.screening_repository.create({ - "id": "older-evidence", "partition_key": "older-evidence", "kind": kind, - "subject": subject_from_document(initial).to_dict(), "state": "pending_scan", - }) - assert_bootstrap_denied(env) - - -def test_inspection_of_an_existing_scan_consumes_before_engine_execution(screened_publication): - env = screened_publication - initial = submit(env) - scan_id = initial["content_screening"]["scan_id"] - policy = env.screening.get_effective_policy(subject_from_document(initial), repository=env.screening_repository) - env.screening_repository.create({ - "id": scan_id, "partition_key": scan_id, "kind": "scan", - "subject": subject_from_document(initial).to_dict(), "state": "pending_scan", - "actor_id": REQUESTER, "policy": policy, "policy_fingerprint": env.screening.hash_payload(policy), - }) - - def engine(*args, **kwargs): - saved = receipt(env) - assert saved[CONSUMPTION]["scan_id"] == scan_id - raise InjectedCrash() - - with pytest.raises(InjectedCrash): - env.screening.inspect_scan( - scan_id, [ContentUnit("unit", "Ordinary content")], REQUESTER, - repository=env.screening_repository, storage=env.screening_storage, engine=engine, - ) - consumed = receipt(env)[CONSUMPTION] - env.source.change(env.target, content_screening=initial["content_screening"]) - env.screening_metadata.documents.pop((scan_id, scan_id)) - assert_bootstrap_denied(env) - saved = receipt(env) - assert saved[CONSUMPTION] == consumed - - -@pytest.mark.parametrize("first", ["content_screening.service", "functions_artifact_publication_readiness"]) -def test_latch_modules_keep_real_cold_imports_below_azure_bootstrap(first): - second = ( - "functions_artifact_publication_readiness" - if first == "content_screening.service" else "content_screening.service" - ) - code = ( - "import importlib, socket, sys\n" - "def blocked(*args, **kwargs):\n" - " raise RuntimeError('Network forbidden during cold import')\n" - "socket.create_connection = blocked\n" - "socket.socket.connect = blocked\n" - f"importlib.import_module({first!r})\n" - f"importlib.import_module({second!r})\n" - "if any(name in sys.modules for name in ('config', 'functions_settings', 'functions_artifact_publication')):\n" - " raise RuntimeError('Readiness or screening initialized application owners')\n" - ) - result = subprocess.run( - [sys.executable, "-c", code], cwd=APP_ROOT, capture_output=True, text=True, timeout=60, - ) - assert result.returncode == 0, result.stderr - - -if __name__ == "__main__": - sys.exit(pytest.main([str(Path(__file__).resolve()), *sys.argv[1:]])) diff --git a/functional_tests/test_public_document_publication.py b/functional_tests/test_public_document_publication.py index 24b018796..1f54ba57c 100644 --- a/functional_tests/test_public_document_publication.py +++ b/functional_tests/test_public_document_publication.py @@ -1,24 +1,25 @@ #!/usr/bin/env python3 """ Functional tests for immutable-target public workspace generated-artifact decisions. -Version: 0.261.183 +Version: 0.261.230 Implemented in: 0.261.134 Decision link opens the V2 public workspace route: 0.261.148 A pending artifact is visible only to the hosting workspace's managers; a reader's review or cancel of one answers as a document that does not exist: 0.261.183 +Generated destinations need no screening reservation; a held legacy destination blocks approval: 0.261.230 The scoped public collaboration routes, the public publication adapter, the -shared canonical artifact-publication engine, the shared screening consume-latch -and revision cleanup all run against conditional local stores and exact source -bytes. The workspace is always taken from the path, so a stale active-workspace -preference can never redirect a publication decision. No Azure, model, provider, -or filesystem artifact processing occurs. +shared canonical artifact-publication engine and revision cleanup all run +against conditional local stores and exact source bytes. The workspace is +always taken from the path, so a stale active-workspace preference can never +redirect a publication decision. No Azure, model, provider, or filesystem +artifact processing occurs. Coverage is deliberately focused on what the *public* wrapper owns: per-role and per-status authorization, requester-only cancel, receipt shaping (applied/queued/unchanged/partial), etag binding, current-revision-only -enforcement, 400 on malformed input, cross-scope isolation, the public -consume-latch identity rules (contract §5), and the read projector's inline +enforcement, 400 on malformed input, cross-scope isolation, approval admission +from the destination's own screening state, and the read projector's inline collaboration actions. The shared engine's own internals are exhaustively covered by the group publication suite and are not re-tested here. """ @@ -50,9 +51,6 @@ from test_support.versioning import assert_app_version_at_least sys.path.insert(0, str(Path(APP_ROOT))) -from functions_artifact_publication_readiness import ( # noqa: E402 - PUBLICATION_SCREENING_CONSUMPTION as CONSUMPTION, -) ROOT = "/api/public-workspaces/public-a/documents" @@ -690,69 +688,31 @@ def test_pending_request_exposes_inline_collaboration_actions_but_no_management_ assert not env.publication_calls["queue"] -# --- Contract §5: the public consume-latch stores operation identity, not a -# boolean, and a reserved scan_id is identity not admission evidence. These -# exercise the real public predicate directly, independent of screening state. - -def _executing_operation(**overrides): - operation = { - "schema_version": 1, "id": "op-1", "action": "approve_artifact", "phase": "executing", - "actor_user_id": REQUESTER, "actor_public_workspace_id": "public-a", - "source_public_workspace_id": "public-a", "document_id": "doc-1", "document_version": 3, - "execution_token": "token-1", - } - operation.update(overrides) - return operation - - -def _latch_document(operation): - return { - "id": "doc-1", "version": 3, "public_workspace_id": "public-a", - OPERATION: operation, - } - +# --- Generated destinations carry no screening reservation. Approval admission +# depends only on the destination's own screening state. -def _latch_receipt(**overrides): - consumption = {"operation_id": "op-1", "fingerprint": "fp"} - consumption.update(overrides.pop("consumption", {})) - receipt = {CONSUMPTION: consumption, "stages": {}} - receipt.update(overrides) - return receipt - - -def test_consume_latch_returns_the_operation_id_when_identity_matches(publication): +def test_public_approval_records_no_screening_reservation(publication): env = publication - retry = env.publication_module._bootstrap_retry_operation_id( - _latch_document(_executing_operation()), _latch_receipt(), REQUESTER, - ) - assert retry == "op-1" - - -@pytest.mark.parametrize("mutation", [ - {"consumption": {"operation_id": "different-op"}}, - {"consumption": {"scan_id": "reserved-scan"}}, - {"stages": {"approval_queue": "started"}}, -]) -def test_consume_latch_rejects_non_identity_or_progressed_receipts(publication, mutation): - env = publication - retry = env.publication_module._bootstrap_retry_operation_id( - _latch_document(_executing_operation()), _latch_receipt(**mutation), REQUESTER, - ) - assert retry is None + submit(env) + response = decide(env, "approve") + assert_receipt(response, env, "approve", "queued", "approved", 202) + saved = receipt(env) + assert not any(str(key).startswith("screening_") for key in saved) + assert "operation_id" not in saved["decision"] + assert not hasattr(env.publication_module, "_bootstrap_retry_operation_id") + assert len(env.publication_calls["queue"]) == 1 -@pytest.mark.parametrize("operation_change", [ - {"phase": "complete"}, - {"actor_user_id": "someone-else"}, - {"source_public_workspace_id": "other-public"}, - {"action": "reject_artifact"}, -]) -def test_consume_latch_rejects_operations_that_are_not_the_executing_approval(publication, operation_change): +@pytest.mark.parametrize("state_value", ["pending_scan", "pending_review", "scan_error"]) +def test_public_screening_hold_on_a_legacy_destination_blocks_approval(publication, state_value): env = publication - retry = env.publication_module._bootstrap_retry_operation_id( - _latch_document(_executing_operation(**operation_change)), _latch_receipt(), REQUESTER, - ) - assert retry is None + submit(env) + env.source.change(env.target, content_screening={"state": state_value, "scan_id": "legacy-scan"}) + response = decide(env, "approve") + assert response.status_code == 409, response.get_json() + assert "request publication again" in response.get_json()["message"] + assert "decision" not in receipt(env) + assert not env.publication_calls["queue"] if __name__ == "__main__": From ed2a15a0dfe8b5e9a8913573e649085fb3b67027 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Sat, 3 Oct 2026 01:42:09 -0400 Subject: [PATCH 2/3] Update screening history and fixture parity tests for direct metadata edits The full functional comparison against the base branch found three tests that still asserted the removed behavior: - test_content_screening_history: a release proof that doesn't match the content still leaves only the screening status; metadata edited after a release now stays available. - test_group_document_fixture_parity / test_public_document_fixture_parity: a released screened document's metadata change returns the ordinary updated receipt (200) instead of queued (202). Also records the base-branch comparison in the fix documentation. Refs #1621 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md | 4 ++++ .../test_content_screening_history.py | 20 +++++++++++++++---- .../test_group_document_fixture_parity.py | 14 +++++++------ .../test_public_document_fixture_parity.py | 17 +++++++++------- 4 files changed, 38 insertions(+), 17 deletions(-) diff --git a/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md index 8445fd7e4..19dba4680 100644 --- a/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md +++ b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md @@ -105,4 +105,8 @@ Existing tests that asserted the removed behavior were rewritten: - `test_content_screening_pipeline.py`: a metadata edit keeps a release, recovery finalizes a release after a metadata edit, and only the source-format rule remains. - `test_group_document_management.py`: a screened metadata edit returns `updated` and keeps the release; a screened tag edit updates chunks but not the release blob. - `test_public_document_publication.py`: approval records no screening reservation, and a held legacy destination blocks approval. +- `test_content_screening_history.py`: a release proof that doesn't match the content still leaves only the screening status, and metadata edited after a release stays available. +- `test_group_document_fixture_parity.py` and `test_public_document_fixture_parity.py`: a released screened document's metadata change returns the ordinary `updated` receipt (200), not `queued` (202). - `test_group_document_publication_screening_bootstrap.py` tested only the removed reservation and was removed. Its import check moved to the new file. + +The 258 functional test files present on both branches were run on the base branch and with this change. No test that passes on the base branch fails with this change; the remaining failures already fail on the base branch for unrelated reasons, such as harnesses that need Azure configuration. diff --git a/functional_tests/test_content_screening_history.py b/functional_tests/test_content_screening_history.py index 3566c7e98..e16c008f0 100644 --- a/functional_tests/test_content_screening_history.py +++ b/functional_tests/test_content_screening_history.py @@ -1,12 +1,13 @@ # test_content_screening_history.py """ Functional regressions for public history and native evidence quarantine. -Version: 0.261.223 +Version: 0.261.230 Implemented in: 0.261.106 Executes the real history route, artifact hydration, and model-history guard against fake Cosmos. Removed metadata aliases and missing release proofs must -not turn old source material into newly authorized evidence. +not turn old source material into newly authorized evidence. Since 0.261.230, +metadata edited after a release keeps the document available. """ import ast @@ -224,14 +225,25 @@ def test_removed_search_metadata_aliases_are_replaced_before_provenance(self): def test_public_metadata_is_status_only_without_matching_release_proof(self): self.scans.documents.clear() + self.document["abstract"] = "UNRELEASED_PRIVATE_ABSTRACT" result = access.public_documents_payload([self.document], "user-1")[0] self.assertFalse(result["content_screening"]["available"]) self.assertNotIn("abstract", result) self.seed_release(self.document) - self.document["abstract"] = "UNAPPROVED_PRIVATE_ABSTRACT" + scan = self.scans.documents[self.document["content_screening"]["scan_id"]] + scan["publication"]["content_fingerprint"] = "a-different-release" result = access.public_document_payload(self.document) self.assertFalse(result["content_screening"]["available"]) - self.assertNotIn("UNAPPROVED_PRIVATE_ABSTRACT", json.dumps(result)) + self.assertNotIn("UNRELEASED_PRIVATE_ABSTRACT", json.dumps(result)) + + def test_metadata_edited_after_release_stays_available(self): + """Metadata edits apply directly and are not screened, so they don't revoke a release.""" + self.seed_release(self.document) + self.document.update({"abstract": "Edited after release", "title": "Edited title"}) + result = access.public_document_payload(self.document) + self.assertTrue(result["content_screening"]["available"]) + self.assertEqual(result["abstract"], "Edited after release") + self.assertEqual(result["title"], "Edited title") if __name__ == "__main__": diff --git a/functional_tests/test_group_document_fixture_parity.py b/functional_tests/test_group_document_fixture_parity.py index db1af1bac..05d3ba7d8 100644 --- a/functional_tests/test_group_document_fixture_parity.py +++ b/functional_tests/test_group_document_fixture_parity.py @@ -1,10 +1,11 @@ # test_group_document_fixture_parity.py """ Per-route shape parity between the M2 group document UI fixtures and the real routes. -Version: 0.261.168 +Version: 0.261.230 Implemented in: 0.261.161 A tag vocabulary conflict is pinned from the etag pre-check and from a lost patch: 0.261.167 A bulk tagging batch or metadata save that meets it is refused whole, with no document written: 0.261.168 +A released screened document's metadata change applies directly and returns the ordinary receipt: 0.261.230 The V2 group Documents explorer mocks the network with three closed HTTP fixtures, which predate the per-route parity rule: @@ -481,18 +482,19 @@ def real_synced_delete_guard(file_sync): return namespace["build_synced_document_delete_guard"] -@pytest.mark.parametrize("scenario", ["updated", "queued", "propagation_incomplete"]) +@pytest.mark.parametrize("scenario", ["updated", "screened", "propagation_incomplete"]) def test_metadata_receipt_parity(management, scenario): - """A saved change names its fields in request order; a screened document's change is queued - (202); a change whose projections failed after the document saved is a coded 500.""" + """A saved change names its fields in request order; a released screened document's change applies + directly and returns the same receipt; a change whose projections failed after the document saved + is a coded 500.""" env = management body = {"title": "Changed title", "keywords": ["alpha", "beta"]} expected, status = metadata_result("document-a", body), 200 - if scenario == "queued": + if scenario == "screened": env.settings["enable_content_screening"] = True env.seed_release(env.source.records["document-a"]) body = {"abstract": "Updated abstract"} - expected, status = metadata_result("document-a", body, queued=True), 202 + expected = metadata_result("document-a", body) elif scenario == "propagation_incomplete": def fail_chunk(**_kwargs): raise StoreFailure() diff --git a/functional_tests/test_public_document_fixture_parity.py b/functional_tests/test_public_document_fixture_parity.py index afa2ca8cc..5637ff3b9 100644 --- a/functional_tests/test_public_document_fixture_parity.py +++ b/functional_tests/test_public_document_fixture_parity.py @@ -1,13 +1,15 @@ # test_public_document_fixture_parity.py """ Per-route shape parity between the M9B public document UI fixtures and the real routes. -Version: 0.261.186 +Version: 0.261.230 Implemented in: 0.261.179 A generated artifact awaiting publication is held for managers and absent for everyone else (decision 27): 0.261.183 Reader downloads: the read fixture models a downloads-off workspace, so a reader's file_downloads_enabled is False and a download the workspace no longer offers is refused with the server's downloads-unavailable sentence: 0.261.186 +A released screened document's metadata change applies directly and returns the ordinary +receipt: 0.261.230 The V2 public Documents explorer mocks the network with three closed HTTP fixtures, which predate the per-route parity rule: @@ -507,9 +509,10 @@ def test_metadata_receipt_parity(manage, scenario): assert_receipt_parity(f"metadata {scenario}", served, real) -def test_queued_metadata_receipt_parity(manage): - """A screened document's change is queued for screening (202). The screening release proof is made - to pass, so the document counts as available, as a released document is.""" +def test_screened_metadata_receipt_parity(manage): + """A released screened document's change applies directly and returns the ordinary receipt (200). + The screening release proof is made to pass, so the document counts as available, as a released + document is.""" env = manage record = env.source.records["same-document"] record["content_screening"] = {"state": "cleared", "scan_id": "scan-same-document", "source_revision": "3"} @@ -525,10 +528,10 @@ def released(document): body = {"abstract": "Updated abstract"} real = raw_json(env, "PATCH", "same-document", body) served = fixture_receipt( - env.fixture, "PATCH", "same-document", response=metadata_result("same-document", body, queued=True), - status=202, body=body, + env.fixture, "PATCH", "same-document", response=metadata_result("same-document", body), + status=200, body=body, ) - assert_receipt_parity("metadata queued", served, real) + assert_receipt_parity("metadata screened", served, real) @pytest.mark.parametrize("mode", ["current_only", "all_versions"]) From 8fc81b523ebd1121d6a53ec3f475c813fe6df80a Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Sat, 3 Oct 2026 02:30:03 -0400 Subject: [PATCH 3/3] Remove the queued metadata receipt from the V2 explorer and UI fixtures Metadata edits now apply directly, so a metadata save never gets a queued (202) receipt. This finishes that change in the client and the tests: - documentOperations.ts accepts a metadata save only when the receipt is updated with HTTP 200; any other receipt, including a legacy queued 202 one, is unconfirmed and keeps the draft. - DocumentExplorer.tsx drops the queued branch that removed the document from the list and showed "Screening is queued"; a confirmed save shows "Metadata saved." and the document stays listed and available. - The group and public metadata routes drop their unreachable 202 branch. - The group and public UI fixtures no longer build a queued receipt. - test_queued_metadata_acknowledgement_keeps_saved_content_held_for_screening becomes test_screened_metadata_edit_applies_directly_and_keeps_the_document_available, and the malformed-receipt test gains a legacy-queued-202 case that fails against the old explorer. - Docs: V2 group document management, the fix doc and the release notes. VERSION stays 0.261.230. Refs #1621 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../route_backend_group_documents.py | 2 +- ...oute_backend_public_document_management.py | 2 +- .../components/documents/DocumentExplorer.tsx | 8 +--- .../v2_ui/src/lib/documentOperations.ts | 8 ++-- .../features/V2_GROUP_DOCUMENT_MANAGEMENT.md | 13 +++--- .../fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md | 25 ++++++++++- docs/explanation/release_notes.md | 3 +- .../fixtures/group_document_management.py | 13 +++--- .../fixtures/public_document_management.py | 13 +++--- ui_tests/test_v2_group_document_management.py | 45 ++++++++++--------- 10 files changed, 80 insertions(+), 52 deletions(-) diff --git a/application/single_app/route_backend_group_documents.py b/application/single_app/route_backend_group_documents.py index fbb635430..2cc3f7dfd 100644 --- a/application/single_app/route_backend_group_documents.py +++ b/application/single_app/route_backend_group_documents.py @@ -633,7 +633,7 @@ def api_scoped_group_document_metadata(group_id, document_id): _group_management_query() changes = validate_metadata_changes(request.get_json(silent=True)) receipt = update_group_document_metadata(get_current_user_id(), group_id, document_id, changes) - return jsonify(receipt), 202 if receipt["status"] == "queued" else 200 + return jsonify(receipt), 200 @bp.route('/api/groups//documents/', methods=['DELETE']) @swagger_route(security=get_auth_security()) diff --git a/application/single_app/route_backend_public_document_management.py b/application/single_app/route_backend_public_document_management.py index d7a721148..909da6b97 100644 --- a/application/single_app/route_backend_public_document_management.py +++ b/application/single_app/route_backend_public_document_management.py @@ -113,7 +113,7 @@ def api_scoped_public_document_metadata(workspace_id, document_id): _public_management_query() changes = validate_metadata_changes(request.get_json(silent=True)) receipt = update_public_document_metadata(get_current_user_id(), workspace_id, document_id, changes) - return jsonify(receipt), 202 if receipt["status"] == "queued" else 200 + return jsonify(receipt), 200 @bp.route('/api/public-workspaces//documents/', methods=['DELETE']) @swagger_route(security=get_auth_security()) diff --git a/application/v2_ui/src/components/documents/DocumentExplorer.tsx b/application/v2_ui/src/components/documents/DocumentExplorer.tsx index 857ec68ba..42f74187b 100644 --- a/application/v2_ui/src/components/documents/DocumentExplorer.tsx +++ b/application/v2_ui/src/components/documents/DocumentExplorer.tsx @@ -1140,14 +1140,10 @@ function ScopedDocumentExplorer({ const changes = changedDocumentMetadata(target, draft); beginMutation('Saving metadata', 1); try { - const status = await captured.adapter.editMetadata(captured.targets[0], changes); + await captured.adapter.editMetadata(captured.targets[0], changes); if (!mounted.current) return; setDialog(null); - if (status === 'queued') { - setDocuments((current) => current.filter((document) => documentId(document) !== documentId(target))); - setInspectedId(null); - toast.info('Metadata saved. Screening is queued; the document remains unavailable until released.'); - } else toast.success('Metadata saved.'); + toast.success('Metadata saved.'); await refreshAll(); } catch (saveError) { if (mounted.current) { diff --git a/application/v2_ui/src/lib/documentOperations.ts b/application/v2_ui/src/lib/documentOperations.ts index c1e7e3308..a6f40f35a 100644 --- a/application/v2_ui/src/lib/documentOperations.ts +++ b/application/v2_ui/src/lib/documentOperations.ts @@ -82,7 +82,7 @@ export interface DocumentOperationAdapter { advertised: boolean; allows: (operation: DocumentOperation, documents?: readonly WorkspaceDocument[]) => boolean; upload: (files: File[]) => Promise; - editMetadata: (document: WorkspaceDocument, changes: DocumentMetadataUpdate) => Promise<'updated' | 'queued'>; + editMetadata: (document: WorkspaceDocument, changes: DocumentMetadataUpdate) => Promise; tagDocuments: (documents: WorkspaceDocument[], action: BulkTagAction, tags: string[]) => Promise; deleteDocuments: (documents: WorkspaceDocument[], options: DocumentDeleteOptions) => Promise; download: (documents: WorkspaceDocument[]) => Promise; @@ -429,21 +429,21 @@ function createOperations( const response = await requestWithStatus(`${base}/${encodeURIComponent(documentId(document))}`, { method: 'PATCH', body: changes }); const result = response.data; const fields = Object.keys(changes); + // Metadata edits apply directly and are never screened, so only an `updated` 200 confirms one. if (!isRecord(result) || result.document_id !== documentId(document) || result[scopeField] !== scope.id || typeof result.message !== 'string' || result.error || !Array.isArray(result.updated_fields) || result.updated_fields.length !== fields.length || !fields.every((field) => Array.isArray(result.updated_fields) && result.updated_fields.includes(field)) - || (result.status === 'updated' ? response.status !== 200 : result.status === 'queued' ? response.status !== 202 : true) + || result.status !== 'updated' || response.status !== 200 || (result.errors !== undefined && (!Array.isArray(result.errors) || result.errors.length > 0))) { throw new Error('The server did not confirm the metadata change for this document. Your draft is kept; refresh before retrying.'); } - return result.status === 'queued' ? 'queued' : 'updated'; + return; } const result = await updatePersonalDocumentMetadata(documentId(document), changes); if (isRecord(result) && (result.success === false || (Array.isArray(result.errors) && result.errors.length))) { throw new Error('Metadata was not fully saved. Your draft has been kept; refresh before retrying.'); } - return 'updated'; }, tagDocuments: async (documents, action, tags) => { const ids = idsFor('tag_documents', documents); diff --git a/docs/explanation/features/V2_GROUP_DOCUMENT_MANAGEMENT.md b/docs/explanation/features/V2_GROUP_DOCUMENT_MANAGEMENT.md index 585e45dda..5300dfef8 100644 --- a/docs/explanation/features/V2_GROUP_DOCUMENT_MANAGEMENT.md +++ b/docs/explanation/features/V2_GROUP_DOCUMENT_MANAGEMENT.md @@ -72,9 +72,12 @@ not introduced here. ## Outcomes and recovery Metadata requests contain only changed allowed fields and are validated before -side effects. Scope-bound receipts distinguish an applied update from an -accepted screening rescan. A queued update is not described as immediately -available content. +side effects. From version **0.261.230**, a metadata edit applies directly, +including on a screened document, because metadata edits are not screened. The +only receipt that confirms a save is `updated` with HTTP 200, and the edited +document stays listed and available. Any other receipt, including the `queued` +(202) receipt screened documents used to get, leaves the save unconfirmed and +keeps the draft. Strict writes claim the source conditionally before downstream projection changes. A conflict or disappeared source does not recreate a deleted record or @@ -150,8 +153,8 @@ On compact screens, Upload stays primary while the **Actions** picker exposes the other eligible commands without shrinking the document viewport. Filters and details retain the existing compact dialogs. -Use the metadata editor for changed fields, review its queued or applied result, -and keep the draft if saving fails. For bulk work, review the per-item outcome +Use the metadata editor for changed fields. A confirmed save applies at once, +and a failed or unconfirmed save keeps the draft. For bulk work, review the per-item outcome before retrying: successful items are not automatically submitted again. Tag rename/delete can retain old vocabulary until unresolved documents or a definition conflict are reconciled. diff --git a/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md index 19dba4680..22a126065 100644 --- a/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md +++ b/docs/explanation/fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md @@ -39,13 +39,23 @@ Files modified: - `application/single_app/functions_public_document_publication.py` - `application/single_app/functions_group_document_management.py` - `application/single_app/functions_public_document_management.py` +- `application/single_app/route_backend_group_documents.py` +- `application/single_app/route_backend_public_document_management.py` - `application/single_app/config.py` +- `application/v2_ui/src/lib/documentOperations.ts` +- `application/v2_ui/src/components/documents/DocumentExplorer.tsx` - `functional_tests/test_content_screening_upload_only_intake.py` (new) - `functional_tests/test_content_screening_jobs.py` - `functional_tests/test_content_screening_pipeline.py` +- `functional_tests/test_content_screening_history.py` - `functional_tests/test_group_document_management.py` - `functional_tests/test_public_document_publication.py` +- `functional_tests/test_group_document_fixture_parity.py` +- `functional_tests/test_public_document_fixture_parity.py` - `functional_tests/test_group_document_publication_screening_bootstrap.py` (removed) +- `ui_tests/fixtures/group_document_management.py` +- `ui_tests/fixtures/public_document_management.py` +- `ui_tests/test_v2_group_document_management.py` ### Marking generated documents @@ -71,7 +81,13 @@ Files modified: - Chunk metadata (title, authors, file name, classification and tags) is now propagated for a screened document while its release is available. Held documents have no released chunks; publication rebuilds them. - `propagate_tags_to_blob_metadata` no longer writes to a screened document's blob. The release pins that blob by ETag and content hash, so rewriting its metadata would have made the released file unreadable. - `_require_release_proof`, `finalize_publication_checkpoint` and `_publication_matches_document` no longer compare the metadata fingerprint. The release still records it. The scan-job reconciliation no longer has a special case for metadata-triggered child scans, because there are none. -- `update_group_document_metadata` and `update_public_document_metadata` always return `updated`, so their routes answer 200. +- `update_group_document_metadata` and `update_public_document_metadata` always return `updated`, and their routes always answer 200. The routes no longer have a 202 branch for a `queued` receipt. + +### V2 Documents explorer + +- `documentOperations.ts` accepts a metadata save only when the receipt is `updated` with HTTP 200. Any other receipt is unconfirmed, including the `queued` (202) receipt screened documents used to get, and the dialog keeps the draft. +- `DocumentExplorer.tsx` has one outcome for a confirmed save: it shows **Metadata saved.** and refreshes the list and details, and the document stays listed and available. Before, a `queued` receipt removed the document from the list and showed "Metadata saved. Screening is queued; the document remains unavailable until released." +- The group and public UI test fixtures no longer build a `queued` metadata receipt. ### Impact @@ -82,7 +98,6 @@ Files modified: ### Known limitations - Release publishes the metadata captured from the inspected copy. A metadata edit made through the API while a document is still held is replaced by that copy when the document is released. The workspace interfaces don't offer metadata edits on held documents. -- The V2 explorer still has an unused branch for a `queued` metadata receipt. It is no longer reachable. ## Validation @@ -107,6 +122,12 @@ Existing tests that asserted the removed behavior were rewritten: - `test_public_document_publication.py`: approval records no screening reservation, and a held legacy destination blocks approval. - `test_content_screening_history.py`: a release proof that doesn't match the content still leaves only the screening status, and metadata edited after a release stays available. - `test_group_document_fixture_parity.py` and `test_public_document_fixture_parity.py`: a released screened document's metadata change returns the ordinary `updated` receipt (200), not `queued` (202). +- `ui_tests/test_v2_group_document_management.py`: `test_queued_metadata_acknowledgement_keeps_saved_content_held_for_screening` became `test_screened_metadata_edit_applies_directly_and_keeps_the_document_available`. After a released screened document's metadata is saved, the explorer shows **Metadata saved.**, the details show the edit, and Chat, Tag, Extract, Download, Delete and Edit stay enabled. `test_malformed_metadata_success_never_discards_the_draft` gained a `legacy-queued-202` case. With the V2 explorer change reverted and the SPA rebuilt, that case fails, because the old explorer accepted the receipt and closed the dialog. - `test_group_document_publication_screening_bootstrap.py` tested only the removed reservation and was removed. Its import check moved to the new file. The 258 functional test files present on both branches were run on the base branch and with this change. No test that passes on the base branch fails with this change; the remaining failures already fail on the base branch for unrelated reasons, such as harnesses that need Azure configuration. + +The V2 change was checked with `npm run typecheck` and `npm run build` in `application/v2_ui`, then with the V2 browser suites against the built SPA and local Playwright: + +- All tests pass in `test_v2_group_document_management.py`, `test_v2_public_documents.py`, `test_v2_personal_document_scope.py`, `test_v2_group_document_collaboration.py` and `test_v2_group_documents.py`. +- `test_v2_content_screening.py` passes except `test_admin_policy_is_discoverable_and_editable_before_citations_are_enabled`, which fails the same way against a build of the base branch's V2 source. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 859f59eaa..29812803b 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -17,7 +17,8 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver * Editing a released document's title, abstract, keywords, authors, classification or tags now applies directly. The document stays available and no new scan starts, so the group and public document metadata APIs return `updated` (200) instead of `queued` (202). * Metadata the model generates after a document is released also applies directly, without a new hold. * A rename still can't change a screened file's extension. Tag edits on a screened document update the document and its search chunks but no longer rewrite the released file's blob metadata, so downloads of that file keep working. - * (Ref: #1621, `functions_documents.update_document`, `content_screening/access.py`, `functions_group_document_management.py`, `functions_public_document_management.py`, [Upload-Only Screening Intake Fix](fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md)) + * In the V2 Documents explorer, saving a screened document's metadata now shows **Metadata saved.**, and the document stays in the list with the edit shown. The explorer no longer removes it with "Screening is queued". A save is confirmed only by an `updated` 200 receipt; any other receipt, including a `queued` 202 one, keeps the draft. + * (Ref: #1621, `functions_documents.update_document`, `content_screening/access.py`, `functions_group_document_management.py`, `functions_public_document_management.py`, `route_backend_group_documents.py`, `route_backend_public_document_management.py`, `documentOperations.ts`, `DocumentExplorer.tsx`, [Upload-Only Screening Intake Fix](fixes/UPLOAD_ONLY_SCREENING_INTAKE_FIX.md)) ### **(v0.261.229)** diff --git a/ui_tests/fixtures/group_document_management.py b/ui_tests/fixtures/group_document_management.py index 44fe7a544..7bfe296ad 100644 --- a/ui_tests/fixtures/group_document_management.py +++ b/ui_tests/fixtures/group_document_management.py @@ -1,7 +1,7 @@ # group_document_management.py """ Closed M2B group document management responses for the real production V2 SPA. -Version: 0.261.195 +Version: 0.261.230 Implemented in: 0.261.129 Every receipt builder, an Owner's rows and the tag list are the real management routes', held to them by functional_tests/test_group_document_fixture_parity.py. @@ -9,6 +9,7 @@ The refusal also answers a bulk tagging batch and, naming the document, a metadata save: 0.261.168 The group-scoped content screening routes, modelled in group_screening.py: 0.261.174 The V2 notification bell's unread count is an expected shell read: 0.261.195 +Metadata edits apply directly and are never screened, so every metadata receipt is `updated`: 0.261.230 Reuse M2A reads, local production assets, request recording, response gates and Azure Playwright connection options. Every management request must consume an @@ -73,7 +74,6 @@ def operation_path(resource, group_id="group-a"): # any other refusal carries its sentence in `error`, and a tag vocabulary conflict also names its # code in `error_code`. METADATA_UPDATED_MESSAGE = "Group document metadata updated." -METADATA_QUEUED_MESSAGE = "Metadata saved and queued for content screening." DOCUMENT_DELETED_MESSAGE = "Group document deleted." TAG_CREATED_MESSAGE = "Group tag created." TAG_MESSAGES = { @@ -106,12 +106,13 @@ def operation_path(resource, group_id="group-a"): ) -def metadata_result(document_id, changes, *, group_id="group-a", queued=False): - """The server names the fields in the order the request sent them.""" +def metadata_result(document_id, changes, *, group_id="group-a"): + """The server names the fields in the order the request sent them. Metadata edits apply directly, + including on a screened document, so the receipt is always `updated`.""" return { - "message": METADATA_QUEUED_MESSAGE if queued else METADATA_UPDATED_MESSAGE, + "message": METADATA_UPDATED_MESSAGE, "document_id": document_id, "group_id": group_id, - "updated_fields": list(changes), "status": "queued" if queued else "updated", + "updated_fields": list(changes), "status": "updated", } diff --git a/ui_tests/fixtures/public_document_management.py b/ui_tests/fixtures/public_document_management.py index 2f5945594..a3d54f63e 100644 --- a/ui_tests/fixtures/public_document_management.py +++ b/ui_tests/fixtures/public_document_management.py @@ -1,11 +1,12 @@ # public_document_management.py """ Closed M3B public document management responses for the real production V2 SPA. -Version: 0.261.179 +Version: 0.261.230 Implemented in: 0.261.133 Server-verbatim propagation failure (`propagation_incomplete`): 0.261.164 Every receipt builder, a manager's rows, the tag list and the download headers are the real management routes', held to them by functional_tests/test_public_document_fixture_parity.py: 0.261.179 +Metadata edits apply directly and are never screened, so every metadata receipt is `updated`: 0.261.230 Reuse M3A public reads, local production assets, request recording, response gates and Azure Playwright connection options. Reads and operations share the immutable @@ -62,7 +63,6 @@ def operation_path(resource, workspace_id="pub-a"): # any other refusal carries its sentence in `error`, and a tag vocabulary conflict also names its code # in `error_code`. METADATA_UPDATED_MESSAGE = "Public document metadata updated." -METADATA_QUEUED_MESSAGE = "Metadata saved and queued for content screening." DOCUMENT_DELETED_MESSAGE = "Public document deleted." TAG_CREATED_MESSAGE = "Public tag created." TAG_MESSAGES = { @@ -86,12 +86,13 @@ def operation_path(resource, workspace_id="pub-a"): DOWNLOADS_UNAVAILABLE_ERROR = "Downloads are unavailable for the selected public workspace." -def metadata_result(document_id, changes, *, public_workspace_id="pub-a", queued=False): - """The server names the fields in the order the request sent them.""" +def metadata_result(document_id, changes, *, public_workspace_id="pub-a"): + """The server names the fields in the order the request sent them. Metadata edits apply directly, + including on a screened document, so the receipt is always `updated`.""" return { - "message": METADATA_QUEUED_MESSAGE if queued else METADATA_UPDATED_MESSAGE, + "message": METADATA_UPDATED_MESSAGE, "document_id": document_id, "public_workspace_id": public_workspace_id, - "updated_fields": list(changes), "status": "queued" if queued else "updated", + "updated_fields": list(changes), "status": "updated", } diff --git a/ui_tests/test_v2_group_document_management.py b/ui_tests/test_v2_group_document_management.py index fbb620d94..8fce7a8d7 100644 --- a/ui_tests/test_v2_group_document_management.py +++ b/ui_tests/test_v2_group_document_management.py @@ -1,7 +1,7 @@ # test_v2_group_document_management.py """ Closed, real-SPA browser scenarios for M2B group document management. -Version: 0.261.174 +Version: 0.261.230 Implemented in: 0.261.129 Coded failures show the server's sentence, delete guards name the conversation, and an archive takes the server's name: 0.261.164 @@ -9,6 +9,8 @@ change documents, or why no one can, and never point at classic: 0.261.167 A manager who can't upload is told why when dropping files: 0.261.168 The group's content screening controls, offered to the members the screening routes accept: 0.261.174 +A screened document's metadata edit applies directly and the document stays available; a queued +metadata receipt no longer confirms a save: 0.261.230 Every scripted receipt is the server's (the builders in fixtures/group_document_management.py, pinned by functional_tests/test_group_document_fixture_parity.py), except the deliberately malformed receipts each robustness scenario names. @@ -628,42 +630,44 @@ def test_metadata_propagation_failure_shows_the_servers_sentence(group_managemen expect(dialog.get_by_role("alert")).not_to_contain_text("document_propagation_incomplete") -def test_queued_metadata_acknowledgement_keeps_saved_content_held_for_screening(group_management_ui): +def test_screened_metadata_edit_applies_directly_and_keeps_the_document_available(group_management_ui): + """Metadata edits are never screened. Saving a released screened document's metadata gets the + ordinary `updated` receipt, and the document stays released: the edit shows in its details and + every action it offered before is still offered.""" ui = group_management_ui + released = {"state": "cleared", "available": True, "finding_count": 0} + ui.record("same-document")["content_screening"] = copy.deepcopy(released) open_documents(ui) dialog = edit_metadata(ui) - body = {"abstract": "New unscreened abstract that must not be released yet."} + body = {"abstract": "Edited after release, with no new screening hold."} dialog.get_by_label(re.compile(r"^Abstract")).fill(body["abstract"]) - held = restricted(changed_record( - ui, "same-document", **body, - content_screening={"state": "pending_review", "available": False, "finding_count": 1}, - )) - held["document_actions"] = ["delete"] reply = ui.queue_operation( - "PATCH", "same-document", body=body, status=202, - response=metadata_result("same-document", body, queued=True), records=[held], + "PATCH", "same-document", body=body, response=metadata_result("same-document", body), + records=[changed_record(ui, "same-document", **body)], ) perform(ui, reply, dialog.get_by_role("button", name="Save", exact=True).click) expect(dialog).to_have_count(0) - expect(ui.page.get_by_text( - "Metadata saved. Screening is queued; the document remains unavailable until released.", exact=True, - )).to_be_visible() - expect(ui.page.get_by_text("Metadata saved.", exact=True)).to_have_count(0) - expect(details_button(ui, "same-document")).to_be_visible() - expect(ui.page.get_by_text(body["abstract"], exact=True)).to_have_count(0) + expect(ui.page.get_by_text("Metadata saved.", exact=True)).to_be_visible() + expect(ui.page.get_by_text(re.compile(r"Screening is queued|remains unavailable"))).to_have_count(0) + details = ui.page.get_by_role("complementary") + expect(details.get_by_text(body["abstract"], exact=True)).to_be_visible() + expect(details.get_by_role("button", name="Edit", exact=True)).to_be_enabled() select_documents(ui, "same-document") - for label in ("Chat", "Tag", "Extract", "Download"): - expect(command(ui, label)).to_be_disabled() - expect(ui.page.get_by_role("complementary").get_by_role("button", name="Edit", exact=True)).to_have_count(0) - expect(command(ui, "Delete")).to_be_enabled() + for label in ("Chat", "Tag", "Extract", "Download", "Delete"): + expect(command(ui, label)).to_be_enabled() + assert ui.record("same-document")["content_screening"] == released + assert ui.record("same-document")["abstract"] == body["abstract"] assert len(ui.operation_requests) == 1 def test_malformed_metadata_success_never_discards_the_draft(group_management_ui): + """Only an `updated` 200 receipt confirms a save. The `queued` 202 receipt that screened metadata + edits used to get before 0.261.230 confirms nothing either, since edits are no longer screened.""" ui = group_management_ui original = copy.deepcopy(ui.record("same-document")) body = {"title": "Unconfirmed metadata draft", "keywords": ["baseline", "unconfirmed"]} receipt = metadata_result("same-document", body) + legacy_queued = {**receipt, "status": "queued", "message": "Metadata saved and queued for content screening."} cases = ( ("message-only", {"response": {"message": METADATA_UPDATED_MESSAGE}}), ("raw-record", {"response": {**original, **body}}), @@ -673,6 +677,7 @@ def test_malformed_metadata_success_never_discards_the_draft(group_management_ui ("extra-field", {"response": {**receipt, "updated_fields": ["title", "keywords", "abstract"]}}), ("duplicate-field", {"response": {**receipt, "updated_fields": ["title", "title"]}}), ("queued-as-200", {"response": {**receipt, "status": "queued"}}), + ("legacy-queued-202", {"response": legacy_queued, "status": 202}), ("updated-as-202", {"response": receipt, "status": 202}), ("empty-error-207", {"response": {**receipt, "errors": []}, "status": 207}), ("html", {"response": b"Sign in to continue.", "content_type": "text/html"}),