diff --git a/application/single_app/Dockerfile b/application/single_app/Dockerfile index 6fcdd26cd..e6dd249ca 100644 --- a/application/single_app/Dockerfile +++ b/application/single_app/Dockerfile @@ -59,13 +59,6 @@ RUN cargo build --release --locked \ && cp Cargo.toml Cargo.lock /onenote-source/wrapper/ \ && cp -R src /onenote-source/wrapper/ -FROM mcr.microsoft.com/azurelinux/distroless/python:3.12 AS onenote-runtime - -COPY --from=onenote-builder /build/target/release/simplechat-onenote-extractor /app/native/onenote_extractor/bin/simplechat-onenote-extractor -COPY --from=onenote-builder /build/THIRD_PARTY_NOTICES.txt /usr/share/simplechat/onenote-extractor/THIRD_PARTY_NOTICES.txt -COPY --from=onenote-builder /build/licenses/ /usr/share/simplechat/onenote-extractor/licenses/ -COPY --from=onenote-builder /onenote-source/ /usr/share/simplechat/onenote-extractor/source/ - FROM mcr.microsoft.com/azurelinux/base/python:3.12 AS builder ARG UID @@ -242,11 +235,18 @@ RUN set -eux; \ ;; \ esac -FROM onenote-runtime +FROM mcr.microsoft.com/azurelinux/distroless/python:3.12 ARG UID ARG GID +# OneNote extractor notices and corresponding source, required when redistributing the image. +# Nothing may be copied under /app before the chowned copy from the builder below creates it, +# so the non-root runtime user owns its working directory. +COPY --from=onenote-builder /build/THIRD_PARTY_NOTICES.txt /usr/share/simplechat/onenote-extractor/THIRD_PARTY_NOTICES.txt +COPY --from=onenote-builder /build/licenses/ /usr/share/simplechat/onenote-extractor/licenses/ +COPY --from=onenote-builder /onenote-source/ /usr/share/simplechat/onenote-extractor/source/ + COPY --from=builder /etc/pki /etc/pki COPY --from=builder /etc/ssl/certs /etc/ssl/certs COPY --from=builder /home/nonroot /home/nonroot @@ -285,6 +285,10 @@ COPY --chown=${UID}:${GID} application/single_app ./ # ever arrives from the v2uibuilder stage. COPY --from=v2uibuilder --chown=${UID}:${GID} /single_app/static/v2 ./static/v2 +# The native OneNote extractor is copied after /app already exists with runtime-user +# ownership. The binary itself stays root-owned: the app user can run it but not replace it. +COPY --from=onenote-builder /build/target/release/simplechat-onenote-extractor /app/native/onenote_extractor/bin/simplechat-onenote-extractor + # Expose port EXPOSE 5000 diff --git a/application/single_app/config.py b/application/single_app/config.py index c13b8dca5..1439ae67d 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.232" +VERSION = "0.261.233" 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/functions_orchestration_rendering.py b/application/single_app/functions_orchestration_rendering.py index d06bf74c1..df04fde74 100644 --- a/application/single_app/functions_orchestration_rendering.py +++ b/application/single_app/functions_orchestration_rendering.py @@ -362,10 +362,34 @@ def __init__(self, service, *, stop=None): self._last_full = None self._last_recheck = None self._due_at = None + self.error_type = None + self.error_cause_type = None + self.error_errno = None def bind(self, claim): self.claim = claim + def record_error(self, error): + """Keep the failure's class names and OS error number, never its message. + + The output code alone can hide the cause: a local file-system PermissionError and + a refused conversation or result access both classify as ``output_access_denied``. + """ + if self.error_type is not None or not isinstance(error, BaseException): + return + self.error_type = type(error).__name__ + chain, current = [], error + while current is not None and len(chain) < 8 and all(current is not seen for seen in chain): + chain.append(current) + current = current.__cause__ + if len(chain) > 1: + self.error_cause_type = type(chain[-1]).__name__ + for item in chain: + number = getattr(item, "errno", None) if isinstance(item, OSError) else None + if type(number) is int: + self.error_errno = number + break + def release_stop(self): """Staged bytes are always committed; a later budget overrun is the owner's to report.""" self.stop = None @@ -448,6 +472,12 @@ def facts(self, outcome=None, failure=None): "status": "raised", "output_code": output_failure(failure)[0] if failure is not None else None, }) + if self.error_type is not None: + facts.update({ + "error_type": self.error_type, + "error_cause_type": self.error_cause_type, + "error_errno": self.error_errno, + }) return facts @@ -931,6 +961,7 @@ def render_attempt(self, output_id, *, claim=None, worker_id=None, stop=None, ob return outcome except Exception as exc: failure = exc + attempt.record_error(exc) raise finally: if observe is not None and attempt.claim is not None: @@ -1020,6 +1051,7 @@ def _render_attempt(self, output_id, claim, worker_id, attempt): except Exception as exc: if observing: raise + attempt.record_error(exc) return self._record_failure( output_id, exc, claim, skip_commit_reconciliation=skip_commit_reconciliation, ) diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index 3295f4203..ca89ea3ce 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -121,6 +121,7 @@ ) from functions_settings import get_settings, is_user_workflows_enabled_for_user from functions_saved_analysis import analysis_artifact_metadata, authorize_analysis_artifact +from functions_temp_files import scratch_file_dir from utils_cache import invalidate_group_search_cache, invalidate_personal_search_cache @@ -3404,7 +3405,7 @@ def open_generated_chat_artifact_stream(artifact, *, check=None): raise RuntimeError("Blob storage client not available") blob_client = blob_service_client.get_blob_client(container=artifact["blob_container"], blob=artifact["blob_path"]) try: - with tempfile.TemporaryFile(mode="w+b", dir=".") as content: + with tempfile.TemporaryFile(mode="w+b", dir=scratch_file_dir()) as content: digest, size = hashlib.sha256(), 0 if check is not None: check() diff --git a/application/single_app/functions_structured_file_renderers.py b/application/single_app/functions_structured_file_renderers.py index dda22c389..9284de908 100644 --- a/application/single_app/functions_structured_file_renderers.py +++ b/application/single_app/functions_structured_file_renderers.py @@ -22,6 +22,7 @@ GeneratedFileExportStream, ) from functions_generated_export_registry import resolve_generated_file_export_format +from functions_temp_files import scratch_file_dir _CHUNK_CHARACTERS = 16384 @@ -441,7 +442,9 @@ def _render_generated_file_source(source, request, *, max_output_bytes, check=No checks = _SourceChecks(source, check) checks.run() with ExitStack() as resources: - stream = resources.enter_context(ClosingExportResource(tempfile.TemporaryFile(mode='w+b', dir='.'))) + stream = resources.enter_context(ClosingExportResource( + tempfile.TemporaryFile(mode='w+b', dir=scratch_file_dir()), + )) try: writer = _OutputWriter(stream, max_output_bytes, checks) count = _RENDERERS[entry.renderer_id](writer, source, request, limits, checks) diff --git a/application/single_app/functions_temp_files.py b/application/single_app/functions_temp_files.py new file mode 100644 index 000000000..95f4a133e --- /dev/null +++ b/application/single_app/functions_temp_files.py @@ -0,0 +1,22 @@ +# functions_temp_files.py +"""Writable scratch locations for short-lived server-side files. + +Import-independent on purpose: renderers and download streams use it without +pulling in configuration or Azure clients. +""" + +import os + + +SC_TEMP_FILES_DIR = '/sc-temp-files' + + +def scratch_file_dir(): + """Return the container's dedicated scratch directory, or None for the platform default. + + Never the process working directory. The container runs as a non-root user, and + the application code directory is not guaranteed to be writable by that user. + """ + if os.path.isdir(SC_TEMP_FILES_DIR) and os.access(SC_TEMP_FILES_DIR, os.W_OK | os.X_OK): + return SC_TEMP_FILES_DIR + return None diff --git a/docs/explanation/features/ONENOTE_INGESTION.md b/docs/explanation/features/ONENOTE_INGESTION.md index 72b8a29ca..68282220b 100644 --- a/docs/explanation/features/ONENOTE_INGESTION.md +++ b/docs/explanation/features/ONENOTE_INGESTION.md @@ -145,8 +145,16 @@ requires correcting the native installation, not re-uploading the same file. ## Build and file layout The Dockerfile has an `onenote-builder` stage with a pinned Rust 1.85.1 image. -The application inherits the `onenote-runtime` stage, containing the native -executable on the same Azure Linux distroless Python 3.12 base. +The final application stage builds directly from the Azure Linux distroless +Python 3.12 image and copies the extractor's third-party notices and +corresponding source from `onenote-builder`. It copies the native executable to +`/app/native/onenote_extractor/bin/` after `/app` already exists with +runtime-user ownership. The final stage must not inherit an intermediate stage +or copy anything under `/app` before that point: a root-owned `/app` stops the +non-root application from writing to its working directory (see +`docs/explanation/fixes/ORCHESTRATION_FILE_RENDER_PERMISSION_FIX.md`). The +executable stays root-owned, so the application user can run it but not +replace it. The React branch retains its independent `v2uibuilder` stage and copies its fresh bundle into that final application image. Publishing the branch is not a deployment; an existing test app needs a rebuild from this branch before it diff --git a/docs/explanation/fixes/ORCHESTRATION_FILE_RENDER_PERMISSION_FIX.md b/docs/explanation/fixes/ORCHESTRATION_FILE_RENDER_PERMISSION_FIX.md new file mode 100644 index 000000000..319d6ea93 --- /dev/null +++ b/docs/explanation/fixes/ORCHESTRATION_FILE_RENDER_PERMISSION_FIX.md @@ -0,0 +1,158 @@ +# Orchestration File Render Permission Fix + +**Version: 0.261.233** + +Fixed in version: **0.261.233**, recorded in +`application/single_app/config.py`. + +Issue: [#1623](https://github.com/microsoft/simplechat/issues/1623). This affects +the React V2 branch (`paullizer-react-v2-ui`) only. The causal commits are not on +`main` or `Development`. + +## Issue + +Chat orchestration could not create downloadable files. A request such as +"create a csv showing the states and their capitals" planned correctly, and the +compose step that prepared the rows completed. The file step then failed, and +the chat showed: + +- `us_states_and_capitals.csv: could not be created.` +- `This operation could not complete.` + +Production telemetry recorded the render attempt as `output_access_denied` +after about 260 ms of rendering. At the time, that code meant source access +could not be confirmed, but the source was general knowledge with no documents +involved. Since 0.261.232 the same code means access to the file's +conversation could not be confirmed. + +## Root cause + +Two changes combined to cause the failure. Neither caused it on its own. + +1. **Container image.** The OneNote native extractor work (2026-09-26) made the + final image stage `FROM onenote-runtime`. That stage copied the extractor + binary to `/app/native/onenote_extractor/bin/`, which implicitly created + `/app` owned by `root:root` with mode `0755`. The later + `COPY --from=builder --chown=${UID}:${GID} /app /app` copies only the + directory's contents, so `/app` itself stayed root-owned. The application + runs as UID `65532` with `/app` as its working directory. +2. **Scratch-file location.** The shared structured file renderer + (`functions_structured_file_renderers.py`) and the verified generated-file + download stream (`open_generated_chat_artifact_stream` in + `functions_simplechat_operations.py`) created their scratch files with + `tempfile.TemporaryFile(dir='.')`, the process working directory. + +Creating a scratch file in `/app` therefore raised +`PermissionError: [Errno 13]`. `output_failure()` classifies every +`PermissionError` as `output_access_denied`, and the original exception was not +logged, so the cause was hidden. + +Inspection of the deployed image layers confirmed the regression: + +| Image | `/app` owner | CSV render | +|---|---|---| +| `20260926-113853` (before the OneNote stage change) | `65532:65532` | Completed | +| `20260930-185829`, `20261002-145750` (after) | `0:0`, mode `0755` | `output_access_denied` | + +### Affected behavior + +- Every orchestration file in the structured formats (CSV, JSON, XML, YAML, + TXT, and Markdown) failed to render. +- Office formats (DOCX, XLSX, PPTX, PDF) render in memory, but downloading any + orchestration-generated file streams it through the same working-directory + scratch file, so those downloads would fail as well. + +## Technical details + +### Files modified + +| File | Change | +|---|---| +| `application/single_app/functions_temp_files.py` | New import-independent `scratch_file_dir()` helper. | +| `application/single_app/functions_structured_file_renderers.py` | The renderer's scratch file uses `scratch_file_dir()` instead of `dir='.'`. | +| `application/single_app/functions_simplechat_operations.py` | `open_generated_chat_artifact_stream` uses `scratch_file_dir()` instead of `dir="."`. | +| `application/single_app/functions_orchestration_rendering.py` | A failed render attempt records its error class names and OS error number in its diagnostics. | +| `application/single_app/Dockerfile` | The `onenote-runtime` stage is removed. The final stage builds from the pinned distroless image and copies the extractor's notices and source directly. The extractor binary is copied after the app code. | +| `docs/explanation/features/ONENOTE_INGESTION.md` | Build layout updated for the new copy location. | +| `docs/reference/logging-tags.md` | New render-attempt diagnostic fields documented. | +| `functional_tests/test_orchestration_file_render_scratch_dir.py` | New regression tests. | +| `functional_tests/test_generated_file_structured_renderers.py`, `functional_tests/test_orchestration_output_lifecycle.py` | Their stream-tracking doubles required `dir='.'`, which pinned the defect. They now require the `scratch_file_dir()` location. | +| `functional_tests/test_workflow_saved_output_artifacts.py` | Supplies `scratch_file_dir` to the download function it loads from source. | +| `functional_tests/test_v2_onenote_uploads.py` | The image-wiring test required the final stage to inherit `onenote-runtime`. It now requires the distroless base and the extractor binary, notices, licenses, and source in the final stage. | + +### Scratch directory + +`scratch_file_dir()` returns `/sc-temp-files` when that directory exists and the +process can write to it. The image creates `/sc-temp-files` for the runtime +user, and document uploads already use it. Otherwise the function returns +`None`, which selects the platform temp directory (`/tmp` in the image, `%TEMP%` +on Windows development machines). It never returns the working directory. + +### Image layout + +The `onenote-runtime` stage is removed. The final stage builds directly from +`mcr.microsoft.com/azurelinux/distroless/python:3.12`, as it did before +2026-09-26. It copies the extractor's notices and corresponding source from +`onenote-builder` to `/usr/share/simplechat/onenote-extractor/`, so the +redistribution notices are unchanged. The chowned copy from the builder stage +then creates `/app` owned by the runtime user. The extractor binary is copied +afterward to the path `functions_onenote.py` expects. The binary remains +root-owned: the application user can run it but cannot replace it. + +Removing the stage also clears a false positive in the Malicious PR Security +Review. That check inspects the whole Dockerfile whenever a pull request +changes it. It requires each `FROM` to name an image with a tag or digest, so it +flagged `FROM onenote-runtime`, a build-stage name. The final `FROM` now names +the pinned image, and the check itself is unchanged. + +### Render diagnostics + +The `[ORCHESTRATION_EXECUTOR] A file render attempt finished.` event now also +includes, for a failed attempt: + +- `error_type`: the class name of the exception that failed the attempt. +- `error_cause_type`: the class name at the end of its explicit `__cause__` + chain, when the error was wrapped. +- `error_errno`: the first OS error number in that chain, when there is one. + +Exception messages, file paths, and identifiers are never recorded. A +successful attempt does not include these fields. The user-facing +classification is unchanged. A file-system `PermissionError` still reports +`output_access_denied`, the same code as an access refusal such as losing +access to the file's conversation. Only the file-system error logs +`error_type=PermissionError` and `error_errno=13`, so the two can be told +apart. + +## Validation + +### Tests + +`functional_tests/test_orchestration_file_render_scratch_dir.py`: + +- `scratch_file_dir()` returns the dedicated directory only when it exists and + is writable. +- No application `tempfile` call passes `dir='.'`, `dir=''`, or + `dir=os.getcwd()`. +- A CSV file renders, commits, and downloads through the real renderer and the + real download stream while any scratch file aimed at the working directory + fails with `EACCES`, as it does in the container. This test fails on the + previous code with `output_access_denied`. +- A failed render logs `PermissionError` and errno `13` without the message or + path. A wrapped failure logs its outer class, root cause class, and errno. +- The Dockerfile's final stage builds from a pinned image rather than another + build stage, creates `/app` only through the runtime user's chowned copy, and + adds the extractor afterward. + +Related suites that pass with the change: `test_orchestration_render_check_cadence.py`, +`test_orchestration_output_lifecycle.py`, `test_generated_file_structured_renderers.py`, +`test_workflow_saved_output_artifacts.py`, `test_orchestration_artifact_publication.py`, +`test_document_provenance_publication.py`, and `test_v2_onenote_uploads.py`. + +### Deployment + +Rebuild and redeploy the image. Each change would fix the failure on its own: +the code no longer writes scratch files to the working directory, and the +rebuilt image gives the runtime user ownership of `/app` again. Both are kept +so a later image-layout change cannot silently break file generation. After +deploying, a failed file render can be diagnosed from the `error_type` and +`error_errno` fields of the render-attempt event. diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 6f54cb128..0e3ff43a4 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,17 @@ 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.233)** + +#### Bug Fixes + +* **Orchestration File Generation Fix** + * Chat orchestration could no longer create downloadable files. A request such as "create a csv showing the states and their capitals" planned correctly and prepared its rows, then the file step failed with "This file could not be created." Every CSV, JSON, XML, YAML, TXT and Markdown file failed this way. Word and PDF files rendered, but downloading any orchestration-generated file used the same failing step, so those downloads would have failed too. + * Two changes interacted to cause this. Since the OneNote extractor was added to the container image, `/app` has been owned by root, while the app runs as a non-root user with `/app` as its working directory. The file renderer and the verified download stream created their scratch files in the working directory, so the operating system refused them. The refusal was reported as `output_access_denied`, a code meant for access refusals, and the real cause wasn't logged. + * Scratch files now go to `/sc-temp-files` when it's writable, and otherwise to the platform temp directory, never to the working directory. The image creates `/app` for the runtime user again. The OneNote extractor binary is added after that and stays root-owned. + * For a failed attempt, the `[ORCHESTRATION_EXECUTOR] A file render attempt finished.` event now also records `sc_error_type`, `sc_error_cause_type` and `sc_error_errno`, so a file-system error can be told apart from an access refusal. Messages and paths aren't logged. + * (Ref: #1623, `functions_temp_files.py`, `functions_structured_file_renderers.py`, `functions_simplechat_operations.py`, `functions_orchestration_rendering.py`, `Dockerfile`, [Orchestration File Render Permission Fix](fixes/ORCHESTRATION_FILE_RENDER_PERMISSION_FIX.md)) + ### **(v0.261.232)** #### Breaking Changes diff --git a/docs/reference/logging-tags.md b/docs/reference/logging-tags.md index db131ede5..7151443d3 100644 --- a/docs/reference/logging-tags.md +++ b/docs/reference/logging-tags.md @@ -376,7 +376,7 @@ file names, file content, prompts, or model responses. | Event message | Severity | Properties | | --- | --- | --- | -| `[ORCHESTRATION_EXECUTOR] A file render attempt finished.` | Information when the file completed; otherwise Warning | `sc_status`, `sc_output_code`, `sc_output_format`, `sc_size_bytes`, and the timing and check counts described below. | +| `[ORCHESTRATION_EXECUTOR] A file render attempt finished.` | Information when the file completed; otherwise Warning | `sc_status`, `sc_output_code`, `sc_output_format`, `sc_size_bytes`, the timing and check counts described below, and `sc_error_type`, `sc_error_cause_type`, and `sc_error_errno` when the attempt failed with an exception. | | `[ORCHESTRATION_EXECUTOR] A step finished after its time budget; its finished result was kept.` | Warning | `sc_failure_code` (`step_timeout` or `run_timeout`), `sc_elapsed_ms`, `sc_step_timeout_seconds`. | | `[ORCHESTRATION_EXECUTOR] Prepared content did not match its declared outputs; asking once more.` | Information | `sc_reason` (`compose_output_retry`), `sc_error_type`, and `sc_execution_code` when the mismatch has one. | | `[ORCHESTRATION_EXECUTOR] Content preparation could not complete.` | Warning | `sc_failure_code`, `sc_execution_code`, `sc_error_type`, and `sc_authority_reason` when a retained source was refused. | @@ -408,6 +408,15 @@ A render attempt records how its time was spent: exception. `sc_output_code` carries the output failure category when there is one; a render stopped by the step time limit reports `failed` with `output_step_time_limit`. +- Since **0.261.233**, an attempt that failed with an exception also records + `sc_error_type`, the exception's class name; `sc_error_cause_type`, the class at + the end of its explicit cause chain when the error was wrapped; and + `sc_error_errno`, the first operating-system error number in that chain. Messages, + paths, and identifiers are never recorded. These fields separate causes that share + an output code: an access refusal, such as losing access to the file's + conversation, and a local file-system `PermissionError` both report + `output_access_denied`, but only the file-system error carries `sc_error_errno` + (`13` for permission denied). To see where a run's file rendering spent its time: @@ -426,7 +435,9 @@ AppTraces verifyMs = toint(Properties.sc_verify_ms), fullChecks = toint(Properties.sc_full_checks), lightChecks = toint(Properties.sc_light_checks), - stopObserved = tobool(Properties.sc_stop_observed) + stopObserved = tobool(Properties.sc_stop_observed), + errorType = tostring(Properties.sc_error_type), + errorErrno = toint(Properties.sc_error_errno) | order by TimeGenerated asc ``` diff --git a/functional_tests/test_generated_file_structured_renderers.py b/functional_tests/test_generated_file_structured_renderers.py index 01a9677be..420477d85 100644 --- a/functional_tests/test_generated_file_structured_renderers.py +++ b/functional_tests/test_generated_file_structured_renderers.py @@ -211,7 +211,8 @@ def opened_streams(monkeypatch): opened = [] def tracked(*args, **kwargs): - assert kwargs.get('dir') == '.' + # Never the working directory: the container's non-root user cannot write /app. + assert kwargs.get('dir') == renderers.scratch_file_dir() stream = original(*args, **kwargs) opened.append(stream) return stream diff --git a/functional_tests/test_orchestration_file_render_scratch_dir.py b/functional_tests/test_orchestration_file_render_scratch_dir.py new file mode 100644 index 000000000..25b2ba851 --- /dev/null +++ b/functional_tests/test_orchestration_file_render_scratch_dir.py @@ -0,0 +1,202 @@ +#!/usr/bin/env python3 +# test_orchestration_file_render_scratch_dir.py +""" +Functional test for orchestration file rendering when the app directory is not writable. +Version: 0.261.233 +Implemented in: 0.261.233 + +This test ensures that generated-file rendering and generated-file downloads never write +scratch files into the process working directory (the container's non-root user cannot +write /app there), that the container image creates /app with runtime-user ownership, and +that a failed render attempt logs its error class names and OS error number so a local +file-system fault can be told apart from an access refusal. +""" + +import ast +import csv +import errno +import io +import json +import os +import re +import tempfile +from pathlib import Path + +import functions_temp_files +from test_orchestration_output_lifecycle import lifecycle, production_modules # noqa: F401 + + +ROOT = Path(__file__).resolve().parents[1] +APP = ROOT / "application" / "single_app" +DOCKERFILE = APP / "Dockerfile" +TEMP_FACTORIES = { + "TemporaryFile", "NamedTemporaryFile", "SpooledTemporaryFile", "TemporaryDirectory", + "mkstemp", "mkdtemp", +} +SKIPPED_DIRECTORIES = {"__pycache__", "node_modules", "static", ".venv", "venv"} +INSTRUCTIONS = ("COPY", "ADD", "RUN", "WORKDIR") + + +def _deny_working_directory_scratch(monkeypatch): + """Fail a temp file aimed at the working directory the way the container's /app does.""" + real = tempfile.TemporaryFile + working = os.path.realpath(os.getcwd()) + targets = [] + + def guarded(*args, **kwargs): + target = kwargs.get("dir", args[6] if len(args) > 6 else None) + targets.append(target) + if target is not None and os.path.realpath(target) == working: + raise PermissionError(errno.EACCES, "Permission denied", os.path.join(working, "tmp-denied")) + return real(*args, **kwargs) + + monkeypatch.setattr(tempfile, "TemporaryFile", guarded) + return targets + + +def _working_directory_temp_calls(): + """Every application tempfile call whose dir argument resolves to the working directory.""" + findings = [] + for path in sorted(APP.rglob("*.py")): + if SKIPPED_DIRECTORIES.intersection(path.relative_to(APP).parts): + continue + tree = ast.parse(path.read_text(encoding="utf-8"), filename=str(path)) + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + name = node.func.attr if isinstance(node.func, ast.Attribute) else getattr(node.func, "id", None) + if name not in TEMP_FACTORIES: + continue + for keyword in node.keywords: + value = keyword.value + relative = isinstance(value, ast.Constant) and value.value in {".", "./", ""} + current = isinstance(value, ast.Call) and getattr(value.func, "attr", None) == "getcwd" + if keyword.arg == "dir" and (relative or current): + findings.append(f"{path.relative_to(ROOT).as_posix()}:{node.lineno}") + return findings + + +def test_scratch_directory_is_the_dedicated_directory_or_the_platform_default(tmp_path, monkeypatch): + print("🔍 Testing scratch files use /sc-temp-files or the platform default, never the working directory...") + monkeypatch.setattr(functions_temp_files, "SC_TEMP_FILES_DIR", str(tmp_path / "missing")) + missing = functions_temp_files.scratch_file_dir() + assert missing is None + + dedicated = tmp_path / "sc-temp-files" + dedicated.mkdir() + monkeypatch.setattr(functions_temp_files, "SC_TEMP_FILES_DIR", str(dedicated)) + chosen = functions_temp_files.scratch_file_dir() + assert chosen == str(dedicated) + + monkeypatch.setattr(functions_temp_files.os, "access", lambda path, mode: False) + unwritable = functions_temp_files.scratch_file_dir() + assert unwritable is None + print("✅ The dedicated directory is used only when it exists and is writable.") + + +def test_application_code_never_creates_temp_files_in_the_working_directory(): + print("🔍 Testing no application tempfile call targets the working directory...") + findings = _working_directory_temp_calls() + assert findings == [], f"Temp files must not be created in the working directory: {findings}" + print("✅ No tempfile call uses dir='.' or the current working directory.") + + +def test_csv_renders_and_downloads_when_the_working_directory_is_not_writable(lifecycle, monkeypatch): + print("🔍 Testing a CSV file renders and downloads with an unwritable working directory...") + targets = _deny_working_directory_scratch(monkeypatch) + output = lifecycle.prepare("csv") + facts = [] + completed = lifecycle.service.render_attempt(output["output_id"], observe=facts.append) + assert completed["state"] == "completed", completed + assert completed["error_code"] is None + rows = list(csv.reader(io.StringIO(lifecycle.download(completed).decode("utf-8")))) + assert rows[0] == ["id", "amount", "enabled"] + assert rows[-1] == ["last", "", "false"] + # One scratch file for the render, at least one more for the verified download stream. + assert len(targets) >= 2 + working = os.path.realpath(os.getcwd()) + assert all(target is None or os.path.realpath(target) != working for target in targets) + assert "error_type" not in facts[0] + print("✅ The file rendered, committed and downloaded without writing to the working directory.") + + +def test_failed_render_logs_error_class_and_errno_without_details(lifecycle): + print("🔍 Testing a failed render records its error class and OS error number only...") + private_path = "/app/tmp-private-scratch-name" + lifecycle.failures["csv"] = [PermissionError(errno.EACCES, "Permission denied", private_path)] + output = lifecycle.prepare("csv") + facts = [] + failed = lifecycle.service.render_attempt(output["output_id"], observe=facts.append) + assert failed["state"] == "failed" + assert len(facts) == 1 + fact = facts[0] + # The user-facing classification is unchanged; the diagnostics now show the real cause. + assert fact["output_code"] == "output_access_denied" + assert fact["error_type"] == "PermissionError" + assert fact["error_errno"] == errno.EACCES + assert fact["error_cause_type"] is None + text = json.dumps(fact) + assert private_path not in text and "Permission denied" not in text + assert all(type(value) in (int, bool, str) or value is None for value in fact.values()) + print("✅ The PermissionError and errno were logged without its message or path.") + + +def test_failed_render_logs_the_root_cause_class_of_a_wrapped_error(lifecycle): + print("🔍 Testing a wrapped failure records the root cause class and its errno...") + + def wrapped(): + try: + raise OSError(errno.ENOSPC, "No space left on device", "/sc-temp-files/tmp-private") + except OSError as exc: + raise RuntimeError("private render detail") from exc + + lifecycle.failures["json"] = [wrapped] + output = lifecycle.prepare("json") + facts = [] + failed = lifecycle.service.render_attempt(output["output_id"], observe=facts.append) + assert failed["state"] != "completed" + fact = facts[0] + assert (fact["error_type"], fact["error_cause_type"], fact["error_errno"]) == ( + "RuntimeError", "OSError", errno.ENOSPC, + ) + assert "private" not in json.dumps(fact) + print("✅ The outer class, root cause class and errno were logged.") + + +def _build_stage_names(dockerfile): + """Stage aliases declared with FROM ... AS; Docker matches them case-insensitively.""" + return { + match.group(1).lower() + for match in re.finditer(r"^FROM\s+\S+\s+AS\s+(\S+)", dockerfile, re.MULTILINE | re.IGNORECASE) + } + + +def test_container_image_creates_app_directory_for_the_runtime_user(): + print("🔍 Testing the image's /app is created by the runtime user's chowned copy...") + dockerfile = DOCKERFILE.read_text(encoding="utf-8") + final_stage = dockerfile.rsplit("\nFROM ", 1)[1] + final_base = final_stage.split(None, 1)[0] + # An inherited build stage can create /app as root first, as the former onenote-runtime stage did. + assert final_base.lower() not in _build_stage_names(dockerfile), ( + f"The final stage must not inherit another build stage: {final_base}" + ) + assert "@sha256:" in final_base or ":" in final_base.rsplit("/", 1)[-1], ( + f"The final base image must be pinned to a tag or digest: {final_base}" + ) + app_copy = final_stage.index("COPY --from=builder --chown=${UID}:${GID} /app /app") + for line in final_stage[:app_copy].splitlines(): + if line.startswith(INSTRUCTIONS): + assert not line.split()[-1].startswith("/app"), f"/app must not exist before its chowned copy: {line}" + code_copy = final_stage.index("COPY --chown=${UID}:${GID} application/single_app ./") + extractor_copy = final_stage.index( + "COPY --from=onenote-builder /build/target/release/simplechat-onenote-extractor " + "/app/native/onenote_extractor/bin/simplechat-onenote-extractor" + ) + assert app_copy < code_copy < extractor_copy + print("✅ /app is created by the chowned copy from a pinned base, and the extractor is added afterward.") + + +if __name__ == "__main__": + import pytest + + raise SystemExit(pytest.main([__file__, "-q"])) diff --git a/functional_tests/test_orchestration_output_lifecycle.py b/functional_tests/test_orchestration_output_lifecycle.py index acf92b4d3..4b7c9e749 100644 --- a/functional_tests/test_orchestration_output_lifecycle.py +++ b/functional_tests/test_orchestration_output_lifecycle.py @@ -58,6 +58,7 @@ ) from functions_orchestration_rendering import OrchestrationRenderingService, execute_render_file from functions_orchestration_results import NamedOutput, OrchestrationResultReader +from functions_temp_files import scratch_file_dir from test_support import offline_bootstrap from test_support.orchestration_results import COLUMNS, ROWS, ResultFixture, complete, source from test_support.orchestration_revisions import AtomicMemoryContainer @@ -1725,8 +1726,8 @@ def test_output_bounds_and_all_private_streams_close_on_failure(lifecycle, monke original = tempfile.TemporaryFile def tracked(*args, **kwargs): - if kwargs.get("dir") != ".": - raise AssertionError("Output scratch files must stay in the project.") + if kwargs.get("dir") != scratch_file_dir(): + raise AssertionError("Output scratch files must use the scratch directory, never the working directory.") stream = original(*args, **kwargs) created.append(stream) return stream diff --git a/functional_tests/test_v2_onenote_uploads.py b/functional_tests/test_v2_onenote_uploads.py index 3eaa4df60..67f2a46de 100644 --- a/functional_tests/test_v2_onenote_uploads.py +++ b/functional_tests/test_v2_onenote_uploads.py @@ -137,13 +137,19 @@ def forbidden(*args, **kwargs): def test_final_image_contains_both_native_runtime_and_built_spa(self): dockerfile = (APP_ROOT / "Dockerfile").read_text(encoding="utf-8") final_stage = dockerfile.rsplit("\nFROM ", 1)[1] - self.assertTrue(final_stage.startswith("onenote-runtime\n")) + # The final stage builds from the distroless image directly, not an intermediate stage + # (0.261.233): an inherited stage created /app as root before the runtime user's copy. + self.assertTrue(final_stage.startswith("mcr.microsoft.com/azurelinux/distroless/python:")) self.assertIn("AS v2uibuilder", dockerfile) self.assertIn("AS onenote-builder", dockerfile) - self.assertIn("AS onenote-runtime", dockerfile) self.assertIn("cargo build --release --locked", dockerfile) - self.assertIn("COPY --from=onenote-builder /build/target/release/simplechat-onenote-extractor", dockerfile) - self.assertIn("COPY --from=onenote-builder /onenote-source/", dockerfile) + for native_copy in ( + "COPY --from=onenote-builder /build/target/release/simplechat-onenote-extractor", + "COPY --from=onenote-builder /build/THIRD_PARTY_NOTICES.txt", + "COPY --from=onenote-builder /build/licenses/", + "COPY --from=onenote-builder /onenote-source/", + ): + self.assertIn(native_copy, final_stage) self.assertIn("COPY --from=v2uibuilder", final_stage) self.assertLess( final_stage.index("application/single_app ./"), diff --git a/functional_tests/test_workflow_saved_output_artifacts.py b/functional_tests/test_workflow_saved_output_artifacts.py index 729cf8438..353fb4656 100644 --- a/functional_tests/test_workflow_saved_output_artifacts.py +++ b/functional_tests/test_workflow_saved_output_artifacts.py @@ -38,6 +38,7 @@ from test_workflow_result_store import FakeBlobService from functions_analysis_access import AnalysisResultUnavailable from content_screening.contracts import DocumentHeldError +from functions_temp_files import scratch_file_dir from functions_workflow_artifacts import ( WorkflowRecordExportSource, authorize_workflow_saved_output_artifact, @@ -129,6 +130,7 @@ def create_message(body): namespace = { "Any": Any, "Dict": Dict, "Optional": Optional, "hashlib": hashlib, "datetime": datetime, "timezone": timezone, "os": os, "uuid": uuid, "tempfile": tempfile, + "scratch_file_dir": scratch_file_dir, "ResourceExistsError": ResourceExistsError, "CosmosResourceExistsError": CosmosResourceExistsError, "CosmosResourceNotFoundError": CosmosResourceNotFoundError, "cosmos_conversations_container": services.conversations, "cosmos_messages_container": services.messages,