Isolated PDF checking for replacement PDF proposals: design plan¶
Status: the plan merged on 10 November (#4683) and Chris said "Build it". P1, the checker service, and P2, the API pipeline, have merged (delivery status at the end of §10). Nothing is deployed or switched on. Where a question is already answered by an owner decision, a standing rule or the code on main, §11.1 records the answer and its source. Everything else has a recommended default, so the plan stands as written unless Chris changes one of the three answers in §11.2.
Where it fits. The notifications restart plan
(/home/chris/workspace/syrf/handover/notifications-implementation-plan-2026-10-24.md, outside the
repository, so not linked; gap M21 and the L9 row) will not merge the
checked replacement PDF work, #3947, until the native PDF
tools are out of the API image or an isolation review is signed off. This plan describes how to take them out:
a separate checking service. It covers where that service runs, how the API talks to it, how the private
virus scanner works, how #3947's code is split, and how it is rolled out.
What changed since the first draft (re-checked against main 6f947c40be on 10 November 2026).
- The service is shared, not L9-only. On 4 November Chris decided that single uploaded PDFs go through one pipeline, this plan's checking service: both the Studies workspace's file uploads (#4751) and L9's proposals (D7 = A in the Studies design decision log).
- The administrator's list already exists. Studies workspace slice 9 is on main: administrators see waiting PDF
proposals in one list and Use or Reject them (
src/services/api/SyRF.API.Endpoint/Controllers/StudyController.cs:GetPdfProposals,UseProposalAsync,RejectProposalAsync). A proposed web address is used as a link and the server never fetches it. A proposed file is refused with409 study-pdf-proposal-file, because SyRF cannot serve a single uploaded file yet. Slice 9 leaves out any proposal that carriesCheckedProposalId, the field #3947 adds: one filter,StudyCorrectionRepository.Waiting()(src/libs/project-management/SyRF.ProjectManagement.Mongo.Data/Repositories/StudyCorrectionRepository.cs), keeps it out of the count, the list and the decision write, and tests pin that. So a checked upload is neither listed nor counted today. Slice 9's decision 9 gives #3947 the job of adding its source value and actions to the reads (slice 9, design decisions 3, 4 and 9, and test 4); in this plan that is P3 (§6, "Changes to slice 9's reads"). - L7 has merged (#4673).
studyAttentionis no longer deployment-managed, and conversations got their own flag that depends on it (src/charts/syrf-common/feature-definitions.yaml, featuresstudy-attentionandreconciliation-conversations). - L8 is still open. #3945 waits on four owner questions, and #3947
is stacked on it. #3947 is 4,242 commits behind main, and 35 of its 76 changed files have also changed on main since
their merge base (counted with
git diff --name-onlyon 10 November). - Nothing from #3947 is on main. There is no
CheckedPdfValidator, nocheckedPdfchart value, noruntime_packagessupport inscripts/generate-dockerfiles.py, and no QPDF or Poppler in any image.
Inputs read. #3947's body and its diff against its base branch (CheckedPdfValidator,
ClamdPdfVirusScanner, PublicPdfDownloader, CheckedPdfProposalsController, CheckedPdfProposalRepository,
the Dockerfile and generator change, and the checkedPdf chart values); its technical plan
docs/features/notification-inbox/checked-pdf-plan.md (on #3947's branch only, not on main, so not linked); #3950
items 5, 6 and 12; .claude/rules/pdf-agent.md; ADR-015; .claude/rules/s3-notifier.md; the PDF agent chart
(its clamd sidecar); .claude/rules/feature-flags.md; the GKE Terraform in camarades-infrastructure; and the
cluster-gitops ApplicationSet. Re-read on 10 November: main's slice 9 code and plan, the Studies decision log (D5 to
D7), feature-definitions.yaml, the PDF agent chart values, cluster-gitops origin/main (c9b25c84: the PDF agent's
staging and production config, the syrf-staging AppProject and the declared NetworkPolicies), and
camarades-infrastructure origin/main (c6e8419, terraform/main.tf).
0. Summary¶
- Recommendation. Build a small dedicated checking service in the cluster, with its own image, which is the only place QPDF and Poppler are installed. It runs as locked-down pods: non-root, read-only filesystem, default seccomp, no service-account token, and no database, broker or cloud credentials. Each environment has its own private ClamAV scanner. The API scans the bytes with ClamAV itself (a plain network client; clamd runs in its own pod), so the virus verdict never passes through the parser. The API keeps the proposal and approval workflow. It stores the bytes in quarantine, and the checker pulls work from a private API port that only the checker can use. The checker only inspects and renders, and reports bounded facts. The API decides from its own scan plus those facts.
- The key finding. The cluster does not enforce NetworkPolicy today. GKE has the network-policy
add-on disabled and no Dataplane V2 (
docs/how-to/bff-session-store-valkey.md; the Terraform cluster resource has neither setting). Until that changes, an "egress-deny" policy is declared intent only. Moving the tools out of the API still removes the main risk: an exploited parser inside the API pod can read the API's database, broker and cloud credentials, and the checker pod holds none of these. But the network-level containment that production should have needs a cluster-wide infrastructure change first (§11.2, question 1). - The MVP. Uploaded PDF proposals, checked by the isolated service, in staging only. The first screen to use it is the Studies workspace's Propose with a file, because it does not wait for L8 (§11.2, question 3). Its administrator list is on main but needs a change to show checked uploads (P3). L9's issue-thread proposals join once L8 is on main. Proposed links keep main's behaviour: they are used as links and never fetched by the server, so the fetch role (§4) is not in the MVP. Production stays off, with no checker deployed there. Main stays promotable throughout.
- The security model in brief. Untrusted bytes are parsed only in pods that hold no SyRF credential, no service-account token and no cloud identity. The API scans with ClamAV first and makes every decision. Anything missing or unverifiable means blocked (§1).
0a. What exists today¶
Checked on 10 November 2026. "Confirmed" means read in the file named; "unconfirmed" means not checked.
| Fact | Source | Status |
|---|---|---|
Administrators Use or Reject waiting proposals in one list, behind studiesWorkspace, with an index from a pre-deploy migration |
StudyController.cs (GetPdfProposalSummary, GetPdfProposals, UseProposalAsync, RejectProposalAsync); StudyPdfProposalsAdmission.cs in SyRF.ProjectManagement.Mongo.Data |
Confirmed |
A proposed file cannot be used (409 study-pdf-proposal-file). A proposal carrying CheckedProposalId is left out of the count, the list and the decision write by one filter, Waiting(); Use and Reject answer 409 study-pdf-proposal-checked for it |
StudyCorrectionRepository.cs (Waiting, GetWaitingStudiesAsync, GetWaitingProposalsAsync, TryDecideAsync); StudyController.cs (UseProposalAsync, ProposalChecked); StudyPdfProposal.cs; pinned by StudyCorrectionRepositoryTests and StudyControllerPdfProposalsTests (the bound fixture) |
Confirmed |
#3947's CheckedPdfProposalRepository reads and writes L8's StudyIssue in its claim, completion and decision transactions (issue checks, StudyIssuePostCapacity, issue posts, StudyIssueUpdated inbox notices); StudyIssue is not on main |
#3947's branch, CheckedPdfProposalRepository.cs (ClaimAsync, CompleteInTransactionAsync, DecideInTransactionAsync, PostAsync) |
Confirmed |
| A proposed or used web address is never fetched by the server; slice 8's availability check fetches only SyRF's own PDF bases | StudyController.cs (SubmitStudyCorrection); slice 8, "What is checked" |
Confirmed |
| Production runs no pre-deploy migrations yet, so a migration-gated admission stays closed there | Studies implementation plan, wave 7 note on migration 0034; ADR-023 | Confirmed in the plan; production state not re-read |
studyAttention is the family switch and is no longer deployment-managed; reconciliationConversations is its own flag that depends on it |
src/charts/syrf-common/feature-definitions.yaml |
Confirmed |
| The PDF agent's clamd is a sidecar with a 2 Gi limit; staging clamd peaked at 1,430 Mi during start-up | src/services/pdf-agent/.chart/values.yaml (clamd:) |
Confirmed |
A Kubernetes staging PDF agent exists but is Paused; production's is disabled; ADR-015 makes ARRNC the host |
cluster-gitops syrf/environments/staging/pdf-agent/values.yaml, syrf/environments/production/pdf-agent/config.yaml; .claude/rules/pdf-agent.md |
Confirmed (the rules file still says no agent runs in Kubernetes) |
The cluster does not enforce NetworkPolicy: no network_policy block and no datapath_provider |
camarades-infrastructure terraform/main.tf (addons_config); docs/how-to/bff-session-store-valkey.md |
Confirmed in Terraform; the live cluster was not read |
| Workload Identity is on, and nodes use the GKE metadata server | terraform/main.tf (workload_identity_config, workload_metadata_config { mode = "GKE_METADATA" }) |
Confirmed |
Declared but unenforced NetworkPolicies exist for Mailpit, Valkey and RabbitMQ; the syrf-staging AppProject allows the NetworkPolicy kind |
cluster-gitops charts/mailpit/templates/networkpolicy.yaml, plugins/local/valkey-*/values.yaml, plugins/helm/rabbitmq/values.yaml, argocd/projects/syrf-staging.yaml |
Confirmed |
Staging and production run in one GKE cluster (camaradesuk), in separate namespaces |
docs/architecture/infrastructure-reference.md (one cluster); cluster-gitops argocd/projects/syrf-staging.yaml and syrf-production.yaml |
Confirmed |
| Which in-cluster endpoints answer without credentials | Not inventoried | Unconfirmed |
| Free node memory for a second clamd in staging | Not checked | Unconfirmed |
1. Goal and threat model¶
Goal. A reporter can propose a replacement PDF, and an administrator can approve the exact bytes after checks pass, with no untrusted bytes or URLs ever handled by a native parser in a process that holds SyRF credentials or can reach SyRF's internal network.
What is untrusted.
| Input | Who controls it | Main risks |
|---|---|---|
| Uploaded PDF bytes (up to 8 MiB) | A member allowed to propose (§3, "Where a proposal comes from"): in the Studies workspace any holder of SubmitPdfCorrections on the project, and in L9 the reporter of an open PDF issue |
Memory-corruption exploits in QPDF, Poppler or ClamAV's own PDF parser; decompression and recursion bombs; very slow inputs |
| Public URL | The same | Server-side request forgery (reaching metadata servers, cluster services, RabbitMQ management, Mailpit and other internal endpoints); DNS rebinding; redirect chains; slow or endless responses; using SyRF to probe third parties |
| Bytes returned by the URL | Whoever runs the remote server | Everything in the first row, plus oversize and compressed responses |
Why the API is the wrong place. In #3947 the API starts qpdf, pdfinfo and pdftoppm as child
processes. The child gets a cleared environment and prlimit bounds, which is good, but it runs as the same
user in the same pod. An exploit can read /proc/1/environ (the Mongo connection string, broker password,
AWS keys and OIDC secrets), can reach everything the API can reach, and lands in the busiest service. The
URL fetch also runs in the API, so a gap in the application-level SSRF checks would expose every internal
service.
What isolation must guarantee.
- No credentials near a parser. The parser child processes hold no credential at all: they start with a scrubbed environment. Their parent, the worker, holds one narrow credential (inspect-role checking calls in this environment) as an environment variable only, never as a file, and is non-dumpable so a child running as the same user cannot read it. The pod holds no database, broker, cloud, identity or email secret.
- No useful network. Once enforcement exists, the parser pod reaches only cluster DNS and the API's private checking port. It cannot reach the scanner. The fetcher reaches only the public internet (with private, link-local, carrier-grade NAT and cluster ranges excluded) and the API's private port.
- Bounded resources. Each check is bounded in bytes, pages, CPU, memory, file size, open files, output
size and wall time. A pod checks one PDF at a time. After any abnormal job (a crash, a timeout or a
limit hit while checking; an API refusal such as
404,401,409or429is not one) the worker exits and the container restarts, so nothing an exploit left behind survives into the next job. A PDF that crashes or hangs the checker twice is failed, so it cannot loop. - Fail closed. The scanner absent, the checker absent, stale signatures, a timeout, or a reply the API cannot verify all mean blocked. Nothing becomes previewable or approvable without a complete, clean, fresh scan, a passing inspection and a matching SHA-256.
- The verdict is the API's. The API runs the virus scan itself, before any parser sees the bytes, and records the result. Only bytes the API has scanned clean are offered for inspection. The checker reports bounded inspection facts, and has no way to report or change a scan result. The API recomputes the final decision from its own scan plus the checker's facts. The checker never writes user-visible text and never touches the database.
- Exact bytes. What the administrator previews and approves is the byte-for-byte object that was scanned and inspected, identified by SHA-256 from quarantine to publication.
Residual risks (accepted, stated).
- A QPDF or Poppler exploit can lie about the inspection of the PDF it is checking (for example, hide active content). It cannot make an infected file clean: the API scans first and records the verdict in its own store, infected or unscanned files are never offered for inspection, and the result contract has no scan field. An administrator must still approve.
- A ClamAV exploit is different. clamd parses the same untrusted bytes and is long-running, so an upload
that compromises it could make every later scan answer
stream: OKuntil clamd restarts. Mitigations: - Restart on any doubt. After any
ERRORreply, timeout or truncated reply, the API sends clamdSHUTDOWNand treats the scanner as unavailable until a freshVERSIONsucceeds. The container's entrypoint also ends clamd after a fixed lifetime (for example 6 hours), so Kubernetes restarts it on a schedule, whatever clamd itself does. While clamd restarts, admission refuses new proposals and any scan in progress is blocked, never clean. - A known-bad canary. The API's scanner readiness check (an internal check that gates admission only,
never a Kubernetes probe) scans the EICAR test string and requires
FOUND. A clamd that answers OK to everything fails it and is treated as unavailable. A targeted exploit could special-case the canary, so this is a tripwire, not a guarantee. - Isolation. clamd runs in its own pod with no credentials, no service-account token and the restricted pod security, accepting connections only from the API pods. A compromise gains a scanner and nothing else.
- Two more layers. Inspection still runs on every file, and an administrator must approve before anything is published.
- A compromised child trying to steal the inspect credential finds no file to read: the credential is only
in the worker's environment. The worker is PID 1 (exec-form entrypoint, no shell wrapper) and marks itself
non-dumpable (
PR_SET_DUMPABLE 0), so its/proc/<pid>/environand memory are unreadable to a same-user child. All capabilities are dropped, so the child cannotptrace. Children start with a scrubbed environment, so nothing is inherited. A stolen credential could not change any scan result. - Until NetworkPolicy is enforced, a compromised pod can reach unauthenticated in-cluster endpoints, and it can send data anywhere on the internet.
- A stolen inspect token is a data-exposure risk. Through
leaseandbytesit could download other proposals' quarantined PDFs, and while egress is not enforced it could send them out. Mitigations: - Short-lived per-pod sessions. The environment-variable secret is only a bootstrap. At start-up the
worker exchanges it (
session) for a session token bound to itscheckerId(the pod name). The worker renews it withsession/renewevery 60 seconds (the renew interval), on its own timer, independent of jobs, and ends it on graceful shutdown. A renewal extends the same session in place: the token does not change, and its expiry moves to 15 minutes ahead. A newsessioncall for the samecheckerIdreplaces the old session, so a container restart in the same pod uses no extra slot. -
When a call is refused. The worker never exits on a refusal. It handles each answer differently:
404(flag off, or a flag-off pod during a rolling deploy): it keeps its token and retries the same call with backoff (15 seconds, up to the 60-second renew interval). It does not open a new session. If the flag is back on within 120 seconds of the last successful renewal, the retried renewal succeeds and the session carries on unchanged.401(session stale or expired): it opens a new session. That cannot trip the replacement alert, because the old session is no longer live. Ifsessionitself answers404, it retriessessionwith the same backoff. So an off period longer than 2 minutes ends with exactly one new session per worker.429fromsession(live-session cap reached): it retriessessionwith backoff (15 seconds, up to 60 seconds). This is how a replacement pod gets its slot once a lost pod's session goes stale. Onlysessionanswers429; no other call does.204fromlease: no work now, or the session's lease rate limit was reached. It keeps its session and retriesleaseafter backoff (15 seconds, up to 30 seconds). The cap keeps an idle, healthy worker polling well inside the 2-minute admission heartbeat (§3, Admission), so being idle never hides the propose action.409onresult(stale or interrupted lease): it drops that job, which the API offers again.
None of these refusals is an abnormal job, so none triggers the container restart in isolation guarantee 3 above (AC 4b). Only a crash, timeout or limit hit in the worker's own checking does. A refusal therefore can never become a crash loop. - A cap on live sessions. Each role may hold at most its configured replica count plus one (for the rolling-update surge) live sessions, a chart value the API reads. Only sessions renewed within the last two renew intervals (120 seconds) count as live. A session that misses two renewals no longer counts towards the cap and is refused for any further call, including
session/renew, so the worker must open a new session. The 15-minute expiry is an outer bound that only a missed staleness check could reach. So when a pod is lost without a graceful shutdown (node loss, eviction), its replacement, which has a new pod name, gets a slot within the 120-second staleness plus onesessionbackoff of starting, and at once if the lost session is already stale by then. Furthersessioncalls beyond the cap are refused. The cap alert measures continuous refusals for a role, until the next successfulsessionfor that role. Its threshold must exceed the staleness window plus the largestsessionbackoff (120 + 60 = 180 seconds), so it is 4 minutes, a clear minute above that worst case: a replacement pod after node loss gets its slot before the alert, but a filled cap raises it. - Rotation by renaming. The bootstrap secret is never changed in place, following the Valkey runbook (§8, Secrets). Rotation is a reviewed GitOps change that introduces a renamed Secret, done when exposure is suspected or on a set review cadence. It is not automatic. - One job at a time. A session can hold one active lease.bytesanswers only for the job currently leased to that session, and only while the lease is valid. Leases are rate-limited per session; a limitedleaseanswers204, like "no work", never429. - Audit logging. Everysession,session/renew,lease,bytes,fetchedandresultcall is logged with the session,checkerId, job id and SHA-256 (never the bytes). Routine renewals raise no alert. There are alerts forsessionrefusals at the cap lasting more than 4 minutes without a successfulsessionfor that role; forsessioncalls that replace a still-live session more often than pods restart (onlysessioncalls count, and replacing a stale session does not); for a second concurrent lease attempt; for abytescall without a matching lease; and for unusual lease rates. - What remains. Anyone holding the bootstrap secret can open sessions up to the cap, or displace a real pod's session by reusing itscheckerId. Each session can then lease one clean-scanned job at a time, download that job's bytes and post inspection facts for it, all audited. They cannot exceed the cap, fetch bytes for a job not leased to their session, act as the other role, reach the scanner, or change any scan result. They can, by filling the cap, stop the real checkers from getting sessions, so checks stop. That fails closed: new proposals are refused or end up blocked, the refused-session alert fires, and nothing is ever made previewable or approvable by it. The fix is to rotate the bootstrap secret.
2. Where the checking runs: options¶
Option A: inside the PDF Agent¶
The PDF Agent already has a clamd scanner and a broker-driven worker model. It is still the wrong host:
- Wrong trust domain. The agent writes into the public PDF root on ARRNC through the CIFS mount. Adding the QPDF and Poppler parsers next to write access to every project's public PDFs makes a parser exploit far worse than it is in the API.
- Not live. The agent is feature-off with activation pending. ADR-015 makes ARRNC its host; a Kubernetes
staging agent exists but is
Paused, and production's is disabled (§0a). L9 would wait on a separate programme's gates. - Different ownership and pipeline. ARRNC containers are reconciled by server-config with maintainer-approved digests, not by cluster-gitops. Its work arrives over the shared production broker.
- It never parses PDFs today. It scans ZIPs with clamd and extracts entries in managed code. Its container has no native PDF tools, and that should stay true.
Reuse its patterns, not its host: the clamd readiness gate (a 72-hour signature-age limit, RELOAD on
stale with a cooldown), freshclam before clamd starts, ConcurrentDatabaseReload no, and "the agent never
holds cloud credentials".
Option B: a dedicated checking service in the cluster (recommended)¶
A new service, pdf-checker (src/services/pdf-checker/, image ghcr.io/camaradesuk/syrf-pdf-checker).
One small .NET worker image is the only image with qpdf (11 or later, for JSON v2), poppler-utils and
util-linux (prlimit). The chart has two workloads in each environment namespace in the MVP, and a third
(pdf-checker-fetch) only if linked PDFs are ever fetched (§4):
| Workload | Does | Reaches (once enforced) | Holds |
|---|---|---|---|
pdf-checker-inspect |
Leases bytes the API has already scanned clean, runs QPDF and Poppler, reports inspection facts | kube-dns, API private port | Inspect-role credential, as an environment variable only |
pdf-checker-fetch (later, optional) |
Leases link proposals, fetches the URL, hands bytes to the API | Public internet on ports 80 and 443 minus excluded ranges, kube-dns, API private port | Fetch-role credential, as an environment variable only |
pdf-checker-clamd |
ClamAV clamd plus freshclam; scans for the API |
ClamAV mirror on 443, kube-dns; ingress only from API pods | Nothing |
Pod security for all three: runAsNonRoot, a fixed non-root UID, readOnlyRootFilesystem: true, a
size-limited emptyDir for scratch, allowPrivilegeEscalation: false, capabilities.drop: [ALL],
seccompProfile: RuntimeDefault, automountServiceAccountToken: false, a dedicated ServiceAccount with no
RBAC, enableServiceLinks: false, and CPU and memory limits. NetworkPolicies ship in the chart.
- For: this matches the GitOps model: the SyRF chart defines it, cluster-gitops enables it per environment,
and production needs no Application at all. It removes every credential from the parser and stays within
one environment, so staging checks never touch production. It has no broker dependency, so it does not
enlarge the MassTransit-to-Wolverine switch-over. #3947's checking code moves almost unchanged. Running
cost is small apart from clamd (about 1.5 to 2 GiB of memory per environment, going by the PDF agent's
sidecar: a 2 Gi limit, and 1,430 Mi at start-up in staging,
src/services/pdf-agent/.chart/values.yaml). - Against: network containment depends on enabling NetworkPolicy enforcement. Until then it rests on pod security and the absence of secrets. Containers share the node kernel, so a kernel exploit escapes. Optional hardening is GKE Sandbox (gVisor) on a small dedicated node pool, which is a Terraform change. It is also a new service, with a CI lane, a chart, image patching and monitoring.
Option C: a serverless sandbox¶
An AWS Lambda container image (managed declaratively by ACK, like the S3 notifier) or GCP Cloud Run (gVisor). Bytes would go through a private S3 or GCS quarantine object.
- For: each execution environment is its own microVM or gVisor sandbox. It costs almost nothing when idle. A VPC-attached Lambda with no NAT and only an S3 gateway endpoint gives enforced egress-deny without touching the cluster.
- Against: the S3 notifier runs without VPC configuration, so today's Lambda shape has uncontrolled internet egress (ADR-015 records this). The VPC, subnets and endpoints the sandbox needs have no declarative owner today, so by our GitOps rule we would have to stop for approval. Cloud Run has no owner in our GitOps setup either. ClamAV in a function loads a database of about 300 MB on every cold start, needs 2 to 3 GB of memory, and needs its own signature-refresh job. It splits checking across two clouds and adds AWS IAM for the API. URL fetching would need a second function with different egress. It is more new infrastructure than the feature justifies.
Recommendation¶
Option B. It removes the credential exposure now, fits the existing chart-plus-cluster-gitops ownership, and keeps each environment separate. Chris chose "one pipeline, this plan's checking service" for single uploaded PDFs on 4 November (D7 = A). Production depends on enforced NetworkPolicy (§11.2, question 1). If Chris rules out enabling enforcement, revisit Option C for production only, because the contract below lets the checker move without changing the API.
3. Message and data contract (API and checker)¶
Principles. The API never runs a native tool. Its ClamAV client is managed socket code that streams
bytes to clamd in clamd's own pod, as #3947's ClamdPdfVirusScanner already does; clamd does the parsing.
Bytes are never inline in a work item or a result: they sit in quarantine and are moved by reference. The checker pulls work from the API over a private
listener. It gets no push, no broker and no database access. The proposal record is the durable work
queue, because #3947 already writes a "checking" claim before any fetch.
Quarantine storage. #3947's CheckedPdfFile record (bytes up to 8 MiB in the environment's MongoDB)
gains an additive state: Quarantined, then Checked, then Published. The record is written when the
proposal is claimed: by the upload request, or by the fetch report for links. Preview needs Checked and a
matching SHA-256. The public accepted-PDF route needs Published. Pending, failed and blocked bytes are
never served to browsers. Moving the bytes to object storage stays a follow-up (#3950 item 6). The checker
sees only an API path, so that move does not change the contract.
Private listener. The API opens a second Kestrel port (for example 8090) for the checking endpoints only,
exposed by a separate ClusterIP Service (syrf-api-checking). That Service is in no ingress. The endpoints
are bound to that port, so the public port answers 404. The checking credential is a generated secret per
role and environment, compared in constant time. During a rotation the API accepts the values from both the
old and the new Secret names (§8). It is used only to open a short-lived session (bound to the pod's
checkerId, renewed every 60 seconds, stale after 120 seconds without renewal, with live sessions capped per
role), and every other call carries the session token. The API takes the role from the credential, never from the request. Each token is limited to its own
calls: a fetch token may call only session/renew, lease and fetched; an inspect token may call only
session/renew, lease, bytes and result. Any other call answers 403, and a lease taken with one role's token cannot be used with the
other's. All checking endpoints answer 404 while the feature flag is off. The chart renders the port and Service only
when checkedPdf.checkingListener.enabled is true (default false), so production renders are unchanged.
The API's own scan. As soon as bytes reach quarantine (in the upload request, or in the fetched call
for a link), the API streams them to clamd, with the 15-second bound, before it offers the job to any
inspector. It reads VERSION first and refuses signatures older than 72 hours or more than 5 minutes in the
future. It then records the scan on the proposal in its own store (additive fields: outcome, engine version,
signature version and date). infected makes the proposal failed. unavailable makes it blocked.
Only clean moves the job to the inspect stage. Nothing the checker sends can create, change or replace
this record.
Endpoints (contract version 1). The fetch role and its fetched call are used only by the optional P9 (§4);
the MVP issues inspect tokens only.
| Call | Allowed role | Request | Response |
|---|---|---|---|
POST /checking/v1/session |
fetch, inspect (bootstrap secret) | checkerId |
200 with a session token and its expiry, replacing any live session for the same checkerId; the role comes from the secret. 429 once the role's live-session cap is reached |
POST /checking/v1/session/renew |
fetch, inspect (session token) | None beyond the token | 200 with the new expiry. It extends the same session in place and never replaces it or issues a new token. 401 if the session is stale (no renewal in 120 seconds) or expired; the worker then opens a new session. It does not count towards the cap and cannot revive a stale session |
POST /checking/v1/lease |
fetch, inspect | checkerId, supported contract versions, tool versions. No role field: the token decides |
204 no work now (including when the session's lease rate limit is reached): retry lease after backoff, never 429; or 200 with jobId (the proposal id), attempt, leaseId, leaseExpiresAtUtc, limits (max bytes, max pages), and either url (fetch) or sha256, size, bytesPath (inspect; offered only after the API's clean scan) |
GET /checking/v1/jobs/{jobId}/bytes |
inspect | leaseId; only for the job currently leased to this session |
The quarantined bytes as application/octet-stream, exact length, with a SHA-256 header |
PUT /checking/v1/jobs/{jobId}/fetched |
fetch | leaseId; body up to 8 MiB; final URL and redirect count |
204. The API hashes and stores the bytes in quarantine, scans them, and moves the job to inspect only if they are clean |
POST /checking/v1/jobs/{jobId}/result |
inspect | leaseId, attempt, sha256, inspection {outcome: passed, refused or incomplete; pages; rendered pages; encrypted; active-content names from a fixed list; PDF header version}, failureCode from a fixed list, durations. There is no scan field |
204 accepted, also for an identical replay; 409 stale lease, job not in the inspect stage, or already terminal |
Bodies are bounded (16 KiB, except bytes) and strictly typed. There are no free-text fields, so a
compromised checker cannot put text into notifications. The API maps failure codes to its own messages.
Unknown extra fields are ignored, so a scan field sent by a forged result has no effect. Unknown enum
values are treated as incomplete, which means blocked.
The API's verdict. The proposal becomes checked only if all of these hold: the lease is current; the
SHA-256 matches quarantine; the API's own recorded scan is clean with a signature date no older than
72 hours and no more than 5 minutes in the future at scan time; inspection is passed; pages are from 1 to 200; rendered pages equal pages;
it is not encrypted; and no active content was found. An infected scan or a refused inspection means
failed (terminal).
Anything else, including disagreement between the checker's outcome and the facts, means blocked. The
inspection facts and tool versions are stored on the proposal (additive fields). Completion then runs one
transaction: the proposal's state and facts, and the file record moved to Checked, plus the origin's own side
effects (below).
Where a proposal comes from. A proposal has an origin, and the pipeline's core (quarantine, scan, lease, verdict, decision) is the same for both:
| Studies workspace (P3, the MVP) | Issue thread (P7, after L8) | |
|---|---|---|
| Who may propose | A holder of SubmitPdfCorrections on the route project (ProjectSubmitPdfCorrectionsPolicy, as SubmitStudyCorrection uses today), on a study that is live in that project, with studiesWorkspace and checkedPdfProposals on |
The reporter of an open PDF issue on the study, as in #3947, with studyAttention, notificationInbox and checkedPdfProposals on |
| Job id (= proposal id) | A stable id from the study id, the proposer's id and the browser's request id, hashed as StudyIssue.PostId is on #3947's branch (SHA-256 of a prefixed string, first 16 bytes) |
As in #3947: from the issue id and the request id |
| Claim writes | The proposal (no issue id), and one StudyPdfCorrection (UploadedToSyrf true) bound to it with CheckedProposalId, in one transaction |
The same, plus #3947's issue checks, history-capacity reservation and the "submitted" issue post and notices |
| Completion and decision side effects | None. The proposer sees the state on the study's card; administrators see it in the list. Notices for this origin are a follow-up, not in the MVP | #3947's issue posts and StudyIssueUpdated inbox notices |
Idempotency. The job id is the proposal id, derived as above, so a browser retry finds the same proposal and
never uploads or fetches again. A lease is taken with a
single conditional update (state = checking, the right stage, and no lease or an expired one), so two API
replicas cannot hand out the same job. A result counts only for the current leaseId and attempt. An
identical replay is a no-op. The first terminal result wins, because of #3947's State == checking guard.
The checks are pure functions of the bytes, so re-running one after a lost lease is safe.
Timeouts. These keep #3947's bounds:
| Stage | Bound |
|---|---|
| Fetch | 30 s in total, 10 s to connect |
| Scan | 15 s |
| Inspect and render | 40 s, with each child process limited by prlimit to 512 MiB of address space, 15 s of CPU, 10 MiB file size and 64 open files |
| Lease | The stage bound plus 15 s |
A lease that expires is offered again once. After two expired or crashed attempts, the job is failed with "checks did not complete". If no checker takes the job within a 10-minute check window, it is blocked with "PDF checking is unavailable". Expiry is applied when the API reads or leases jobs, so no new background worker is needed.
Expiry and the feature flag. The proposer's card in the Studies workspace and L9's issue thread still read proposals while the flag is off, so expiry must not run on those reads. Three rules:
- Expiry pauses while the flag is off. Expiry and attempt counting run only in flag-on code paths
(
lease,result, and proposal reads while the flag is on). Reads while the flag is off show the stored state ("Checks paused") and write nothing to proposals. - The check window restarts when the job is offered again. The API keeps one small activation record
per environment with
onSinceUtcand anoffmarker. The record is metadata, not proposal state. The 10-minute window and lease expiry are measured from the latest of the claim, the last offer andonSinceUtc. The off transition is recorded by both of these, so it does not depend on anyone reading a thread: - API start-up (the flag-change path).
checkedPdfProposalsis deployment-managed, so its value changes only by a rollout, and every API pod with the checking listener enabled reads it at start-up. A pod that starts with the flag off marks the record off. A pod that starts with it on, and finds the record marked off, setsonSinceUtcto now and clears the marker. If the flag ever becomes runtime-overridable, the override write path must do the same in the same request. - Checker calls while off. Each
session,session/reneworleasecall that answers 404 because the flag is off also marks the record off (idempotent, one small write).
With the checker scaled down and nobody reading threads, the start-up mark still records the off period.
During a rolling deploy, mixed pods may mark off and on more than once. Each extra transition only gives
jobs a fresh window, which is safe. The first flag-on request of any kind after the marker is set also
resets onSinceUtc, as a backstop.
3. A flag-off interruption is not an attempt. A lease issued before onSinceUtc is treated as
interrupted: the job is offered again with the same attempt number and a fresh window. Only a lease that
expires, or a crash reported, while the flag stayed on counts towards the two attempts.
Admission. A new proposal is refused before any claim (503 pdf_checking_unavailable, and the propose
action is hidden with a one-line reason) unless checkedPdfProposals decides on and the lease index exists.
That check copies StudyPdfProposalsAdmission on main: the flag is decided on every call (a failed decision is
off), and the index is checked with NotificationAdmissionChecks.IndexesReadyAsync; a positive answer is kept for
the process, and a negative one is asked again on every call. While the index is missing, lease answers 204.
A proposal is also refused when the API's last scanner VERSION check (cached for up to 60 seconds) failed or
was stale, or when no checker of the needed role has leased or polled in the last 2 minutes. Each poll records a heartbeat. A poll is a lease call on a live session, whatever it answers
(200 or 204); a refused session call is not a poll and records nothing. So with one replica, node loss
leaves a window in which admission refuses new proposals, from the loss until the replacement pod has started,
holds a session and leases. Kubernetes replaces a pod on an unreachable node only after the node-monitor grace
period and the default 300-second unreachable toleration, so the whole window is roughly 5 to 6 minutes. Measured
from the moment the replacement pod starts, it is at most one 60-second session backoff, because by then the
lost session is long stale. That fails closed: nothing becomes previewable or approvable. The cap alert is
unaffected, since the replacement's session call is not refused at the cap. A short tolerationSeconds on the
checker Deployment would shorten the window, but it would be a new chart value with no SyRF precedent, so it is
not planned. Per-reporter and per-environment caps (for example 3 checking
proposals per reporter and 50 per environment) replace #3947's in-process semaphore. Nothing is ever
treated as clean because a component is missing: scanner or checker absent = blocked stays the rule.
4. URL fetching¶
Not in the MVP, and optional. On main a proposed web address is stored in normalised form and, when an
administrator uses it, becomes the study's link. The server never fetches it (StudyController.cs,
SubmitStudyCorrection and UseProposalAsync). L9 keeps that behaviour for links (§11.1, item 4). The rest of this
section is the design for a later slice, built only if Chris asks for linked PDFs to be copied into SyRF and checked.
Where. In pdf-checker-fetch, a separate pod from the parser. It is managed .NET code with no native
parsing, so its risk is SSRF, not code execution. It holds no SyRF secret except the fetch-role credential.
The API keeps only a syntax check for the URL before claiming.
Application-level controls (kept from #3947's PublicPdfDownloader):
httporhttpsonly, on default ports; no user information or fragment; at most 2,048 characters.- Every resolved address must be public: IPv4 private, loopback, link-local (including the 169.254.169.254 metadata server), carrier-grade NAT, benchmarking, documentation and multicast ranges are refused, and IPv6 must be ordinary global unicast. The connection is pinned to the checked addresses, which defeats DNS rebinding.
- No proxy, no cookies, no automatic redirects. At most 3 redirects, each validated and resolved again.
- No content encoding.
Content-Lengthand the streamed body are both capped at 8 MiB. The body must start with%PDF-; the inspector checks this again. - Additions: a response-header size limit, a fixed SyRF user agent, no referrer or authorization headers, and TLS validation on.
Network-level controls (once enforced). Egress from the fetch pod is allowed only to 0.0.0.0/0 on
ports 80 and 443 minus 10.0.0.0/8, 172.16.0.0/12, 192.168.0.0/16, 100.64.0.0/10, 169.254.0.0/16
and 127.0.0.0/8 (the GKE pod and service ranges sit inside 10.0.0.0/8; confirm before writing the
policy), plus kube-dns and the API's private port. The pods use the GKE metadata server with no Google
service-account binding, so even a metadata request yields no useful token.
Abuse bounds. Only a member allowed to propose (§3, "Where a proposal comes from") can propose a link. The per-reporter cap applies, there is one fetch per proposal and none on retry, and redirects are recorded with the result.
5. The private ClamAV scanner¶
- Owner. The SyRF
pdf-checkerchart defines it. cluster-gitops enables it per environment (staging first). It is onepdf-checker-clamdDeployment and ClusterIP Service per environment namespace, never shared between staging and production. The API is its only client: the API scans, so the virus verdict never passes through the parser pod. It is not a sidecar, because freshclam needs internet egress and the parser pod must not have it. It runs with the same pod security as the checker, because clamd is itself a native parser of untrusted bytes. Ingress is allowed only from the API pods. The image isclamav/clamavpinned by digest (the PDF agent pins tag1.4; digest is stricter). - Signature freshness. freshclam runs once before clamd starts, then on its normal schedule.
ConcurrentDatabaseReload noavoids holding two databases in memory. Readiness fails when the signatures are older than 72 hours, with oneRELOADattempt per 5-minute cooldown, as in the PDF agent's readiness gate. The API also readsVERSIONbefore every scan, records the signature date with the verdict, and refuses stale dates on its own, so a stale scanner can never produce a clean verdict. Monitoring warns at 24 hours. - Restarts. The API sends
SHUTDOWNafter anyERROR, timeout or truncated reply. The entrypoint ends clamd after a fixed lifetime (for example 6 hours), so it also restarts on a schedule. The API's scanner readiness check includes an EICAR canary that must returnFOUND(§1, residual risks). - Scanner availability gates admission only. It never feeds the API pod's Kubernetes readiness or liveness probes. A clamd outage or restart makes new PDF proposals refused or blocked; it never takes API pods out of service.
- Limits.
StreamMaxLength,MaxFileSizeandMaxScanSizeare each set to 10 MiB, just above the 8 MiB cap.MaxRecursionandMaxFilesare bounded.AlertExceedsMax yesensures a file that hits a limit is reported, not passed as clean.
Verdict semantics.
| clamd says | Scan outcome | Proposal | Reporter sees |
|---|---|---|---|
Fresh VERSION, then exactly stream: OK |
clean |
Continues to inspection | (nothing yet) |
Anything ending FOUND, including limit heuristics |
infected |
Failed, terminal | "This file could not be confirmed safe. The original PDF is unchanged." |
Not configured, refused, timed out, stale or future signatures, size limit exceeded, ERROR, or a truncated reply |
unavailable |
Blocked | "Virus checking is unavailable. This proposal cannot be previewed or approved." |
6. How #3947's code maps onto this¶
All of #3947's code is on its branch only; none of it is on main.
Stays in the API, amended in the API pipeline PR (P2 in §10):
CheckedPdfProposalsController: list, upload, preview and decision (its link action is not carried; links stay on main's slice 9 path, and a fetched link would be P9). Submit now claims, writes the quarantine record, scans an upload with clamd (bounded to 15 s), and returns with state "checking" (or failed or blocked from the scan). It no longer calls the downloader or validator. The in-processSemaphoreSlim(2)is replaced by the admission caps and the heartbeat gate.- The
CheckedPdfProposalandCheckedPdfFileaggregates (new additive fields: check stage, attempt, lease, inspection facts, file state, and an origin;IssueIdbecomes optional),StudyPdfCorrection.BindCheckedProposal(keeping slice 9's field nameCheckedProposalId, which main already reads),AcceptedStudyPdfsController, and the nginx upload location. CheckedPdfProposalRepository, reworked, not carried as is. On #3947's branch every transaction reads or writes L8'sStudyIssue(§0a), which is not on main. P2 keeps the transaction shapes (the claim's study snapshot and correction bind, completion's state and file, the decision's compare-and-set of the study's PDF, the correction's status and the file's publication) and moves the issue checks,StudyIssuePostCapacity, the issue posts and the inbox notices behind the origin (§3), so the Studies origin has none of them. P7 adds the issue origin back on top of L8.ClamdPdfVirusScannerandClamdPdfVirusScannerTests:VERSIONfreshness and chunkedINSTREAM. This is managed socket code; clamd parses the bytes in its own pod. The scan result moves onto the proposal record. Consider sharing a scanner library with the PDF agent'sClamdVirusScannerlater; not in the MVP.PublicPdfDownloader.ValidUri(syntax only) andReadBoundedAsync(bounded upload copy). Both are managed code.- New: the private checking listener and endpoints with per-role token limits, the scan step in the upload
and
fetchedpaths, lease, report and expiry logic, the activation and heartbeat records, and a pre-deploy migration (ADR-023) for the lease-query index on the proposal collection. - Web: the proposal card shows "Checking" and refreshes by bounded polling, and in L9's issue thread also when the inbox hint arrives (the Studies origin sends no notice, §3). This rides with the Material 3 rework that the stack's screens already need.
- Changes to slice 9's reads (P3). Today one filter,
StudyCorrectionRepository.Waiting(), serves both the reads and the decision write, and it leaves out every proposal withCheckedProposalId. P3 splits it: - Unchanged (decision side):
Waiting()itself andTryDecideAsync, so slice 9's Use and Reject and the legacy Approve and Reject keep refusing a checked upload (409 study-pdf-proposal-checked). A checked upload is decided only through the checked decision.StudyPdfProposal.Waitingkeeps meaning "decidable here". - Changed (read side): a new read filter,
ProjectIdandPendingwithout theCheckedProposalIdclause, used byGetWaitingStudiesAsyncandGetWaitingProposalsAsynconly whilecheckedPdfProposalsis on. While it is off, both reads useWaiting()exactly as today, so the flag-off behaviour is unchanged. - New source value:
StudyPdfProposalSourcegainsCheckedUpload(append-only, as its comment requires), chosen when the document carriesCheckedProposalId.StudyPdfProposalDtogains the check state (additive). The Studies list shows it with its state, Preview, and the checked Approve and Reject. - Index:
ProjectId_1_Status_1_StudyId_1serves the new read filter better than today's (no residual clause), and the decision filter still starts from_id. P3's tests assert this withexplain. - Tests that change: in
StudyCorrectionRepositoryTests,OnlyWaitingProposalsInTheProjectCount_AndEachStudyOnceandTheProposalsReadIsWaitingOnlyForTheNamedStudies_NewestFirst_WithTheirSourcekeep their current assertions for the flag-off read and gain a flag-on case where the bound proposal is counted and listed asCheckedUpload. InStudyControllerPdfProposalsTests,TheProposalsReadListsWaitingProposalsNewestFirstWithSourceAndProposerNameandTheCountIsWaitingProposalsOnLiveStudiesOfTheProject_AndADeletedStudyCountsAgainAfterRestoregain flag-on twins that include theboundfixture. Slice 9's test 4 in the Studies implementation plan is reworded in the same PR ("not one carryingCheckedProposalIdwhilecheckedPdfProposalsis off"). - Tests that stay unchanged:
ADecisionMissesAnotherProjectOrStudyAndACheckedProposalandAProposalBoundToACheckedUploadReadsAsChecked_ThoughTheClassHasNoSuchMember(it assertsWaitingis false, which still holds), and slice 9's Use and Reject refusal tests. - Unconfirmed: how slice 8's availability check classifies the accepted-PDF route's address once a checked upload is used. P3 decides and tests it.
Moves to pdf-checker:
CheckedPdfValidator: QPDF JSON v2 active-content inspection,pdfinfo, rendering every page withpdftoppm, theprlimitwrapper, the cleared child environment and the bounded output readers. It also picks up #3950 item 12's prompt child termination.PublicPdfDownloaderandPdfDnsResolver: the fetch role.- Their tests:
CheckedPdfValidatorTests, the downloader address-range tests, and the four renderer tests that run inside the image. - New in the worker: the credential read once from the environment at start-up,
PR_SET_DUMPABLE 0, children started with a scrubbed environment, and exit after any abnormal job so the container restarts.
Deleted:
- The
apt-get install poppler-utils util-linux qpdfline from the API Dockerfile. The native tools must never be added to the API image. A guard test asserts that the generated API Dockerfile installs no runtime packages. #3947'sruntime_packageschange toscripts/generate-dockerfiles.pyis not on main; ifpdf-checkeruses the generator, it adds that support for its own image only. - The
Program.csregistrations for the validator, downloader and DNS resolver. The scanner registration stays.
Carried: #3947's checkedPdf clamd host and port entries for env-mapping.yaml and _env-blocks.tpl are added
to the API, because the API is clamd's client. They render only when checkedPdf values are set, so the
production API render is unchanged.
7. Flags and rollout¶
- New flag (answered in §11.1, item 3).
checkedPdfProposals: a Release flag, default off everywhere, deployment-managed, added withpnpm feature:new. It gates the pipeline: admission, the checking endpoints, preview and the checked decision. It gives PDF checking its own kill switch. - Surface flags. Each screen that offers a checked upload also needs its own flag:
studiesWorkspacefor the Studies workspace, andstudyAttentionandnotificationInboxfor L9's issue thread (as in #3947). L7 liftedstudyAttention's deployment-managed pin, so it is no longer a constraint here. - Off means the behaviour from before the flag. There is no way to propose a file: the Studies workspace's Propose takes a web address only, as on main today, and the issue thread has no propose action. File propose, checked decision, preview and every checking endpoint answer 404, so no leases are handed out and the checker idles. Slice 9's list and count leave checked uploads out, exactly as today (§6). The proposer's card and L9's issue thread show saved proposals read-only, with a note that checking is switched off. Nothing changes state while off, and no deadline expires.
- Round trip (off, on, off, on). Data from each state survives each switch. A proposal still checking when the flag went off is offered again when it comes back on, with a fresh check window and the same attempt count. It is not blocked for the time spent off (the three expiry rules in §3). Checked proposals can be decided again. Approved PDFs stay published in every state, because publication is part of the study's PDF link, not the flag. This needs one test that creates data in each state.
- Independent fail-closed gates. The flag, a working scanner, and a live checker heartbeat. With the flag on and no scanner or checker deployed, as in production today, admission refuses.
Main stays promotable.
- The API image gains no native tools.
- New collections and fields are additive and start empty. The index goes through a pre-deploy migration with a Down step.
- No hosted service is added. The only start-up work is the small activation-record write in §3, and it runs only where the private checking listener is enabled, so production's API starts exactly as today.
- The private listener renders only when its chart value is on, so production's API render is identical (verify by rendering).
- The
pdf-checkerchart exists in the repo, but there is no production cluster-gitops config, so production has no Application. - The contract is versioned and additive. The checker and API negotiate a version at lease time, so they can deploy in either order.
- Admission needs the lease index as well as the flag (§3, Admission), and production runs no pre-deploy migrations yet (§0a). So even a mistaken flag-on there leaves the admission closed until the index exists (AC 19).
Rollout order. Staging only:
- The checker image and chart merge, deployed nowhere.
- The API pipeline and the first screen merge with the flag off everywhere.
- cluster-gitops enables clamd and the inspect role in staging, then the checking listener.
- Chris approves switching the flag on in staging, uploads only.
- Administrators' Add and Replace with a file, and L9's issue-thread proposals once L8 is on main, follow on the same pipeline. The fetch role is built only if Chris asks for linked PDFs to be copied.
Production is a separate decision after the prerequisites in §8 and an operator security review.
8. Operational prerequisites (written down, not executed)¶
| Area | Prerequisite | Owner |
|---|---|---|
| GitOps | syrf/services/pdf-checker/config.yaml and values; syrf/environments/staging/pdf-checker/config.yaml with service.enabled; no production entry; preview kubernetesApplication: false at first |
cluster-gitops |
| Chart and image | src/services/pdf-checker/.chart (the inspect and clamd Deployments, and the fetch Deployment only with P9; Services, NetworkPolicies, PodMonitoring); .github/services.json; docs/architecture/dependency-map.yaml; GitVersion; CI build and test lane on juniper-ci; base-image patch cadence (rebuild at least monthly for Debian security updates) |
syrf |
| NetworkPolicy enforcement | Today networkPolicyConfig is disabled with no Dataplane V2. Enabling Calico enforcement on the existing Standard cluster is expected to recreate the nodes. It also starts enforcing the policies already in cluster-gitops (Mailpit, Valkey, RabbitMQ), which must be audited first. Whether Dataplane V2 can be switched on in place must be checked against current GKE documentation; this plan assumes it cannot |
camarades-infrastructure (Terraform), with an audit in cluster-gitops |
| Optional hardening | A GKE Sandbox (gVisor) node pool and RuntimeClass for the checker pods, decided at the production review (§11.1, item 7) | camarades-infrastructure |
| Capacity | About 2 GiB of memory for staging's clamd, and a little for the inspect pod. Node headroom is unconfirmed | cluster-gitops, checked before P4 |
| Secrets | Fetch and inspect bearer secrets per environment, generated by an External Secrets generator (the Valkey pattern: no human handles them). Each checker pod gets its own role's secret only as an environment variable (secretKeyRef), never as a mounted file or volume. It is a bootstrap secret, exchanged for short-lived sessions that are renewed every 60 seconds (§1, §3). Values are CreatedOnce and never changed in place. ESO keeps no previous value, and a secretKeyRef env var is fixed when the container starts. So rotation follows the Valkey runbook (docs/how-to/bff-session-store-valkey.md, "Rotating a password") and introduces a renamed Secret. The steps run in this order, each a reviewed cluster-gitops change that waits for its rollout: (1) add the new ExternalSecret (for example pdf-checker-inspect-v2) beside the old one; (2) add the new name to the API's accepted list for that role, so the API reads both names, and wait until every API pod runs with both; (3) point the checker Deployment's secretKeyRef at the new name and wait until every checker pod has rolled; (4) after the overlap (every old pod gone and its sessions expired, at least 15 minutes), remove the old name from the API's list and wait for that rollout; (5) delete the old ExternalSecret. At every step the API accepts every value a running checker pod holds, so no pod is left holding only an unknown value. The API gets both roles' values, to verify them. The checker gets no other secret |
cluster-gitops (ESO) |
| Network policy content | Inspect: egress to kube-dns and the API private port only (no route to clamd). Fetch: egress to the public internet minus excluded ranges, kube-dns and the API private port. clamd: ingress from API pods only, egress to 443 and DNS. API private port: ingress from checker pods only | syrf chart; cluster-gitops for values |
| Monitoring | Metrics: signature age and scan outcomes (from the API), jobs by outcome and failure code, lease backlog and age, check duration, child kills and OOMs, container restarts after abnormal jobs, heartbeat age. Alerts: signature age over 24 hours, no heartbeat for 5 minutes while the flag is on, a spike in blocked checks, and the session and lease alerts in §1 (session refusals at the cap lasting more than 4 minutes, session calls replacing still-live sessions more often than pods restart, a bytes call without a matching lease; routine renewals raise none). Logs carry the job id and SHA-256 only, never URLs with query strings or bytes |
syrf chart (PodMonitoring), plus an alert-rule owner to confirm |
| Retention | How long quarantined, failed and blocked bytes are kept (infected bytes should be purged quickly) is part of the open retention gap for notifications | Follow-up |
9. Acceptance criteria¶
MVP criteria: 1 to 10, 12 to 15, and 17 to 20. Criterion 11 belongs to the optional fetch slice, and 16 to production. Each slice's PR states which criteria it meets and how they were checked.
| # | Criterion | How to verify |
|---|---|---|
| 1 | The API image has no qpdf, pdfinfo, pdftoppm or prlimit |
A guard test on the generated API Dockerfile, plus docker run --entrypoint sh syrf-api -c 'command -v qpdf pdftoppm pdfinfo' finding none |
| 2 | The API never starts a child process for PDF work | An architecture test: no Process.Start in API PDF code, and no CheckedPdfValidator type in the API assembly |
| 3 | The checker pod spec is restricted: non-root, read-only root, no capabilities, RuntimeDefault seccomp, no service-account token, no service links, exec-form entrypoint so the worker is PID 1 | Helm unit tests on the rendered Deployments; an image test that PID 1 is the worker |
| 4 | The checker gets no database, broker, cloud or identity secret, and its one role credential arrives as an environment variable only, with no secret volume or file | A Helm test listing every env var, volume and mount: exactly one secretKeyRef env var, no secret volumes |
| 4a | A parser child cannot see the credential: its environment is scrubbed, and the worker's /proc/<pid>/environ is unreadable to it |
A test inside the checker image that runs a probe in place of qpdf and asserts it finds no credential in its own environment or in the worker's /proc entries |
| 4b | After a crash, timeout or limit hit, the worker exits and the container restarts before the next job | A checker test with a tool stub that hangs, crashes and exceeds limits; assert the exit code and that no further lease was taken by that process |
| 4c | clamd restarts after any ERROR, timeout or truncated reply, and on a fixed lifetime. The scanner counts as unavailable until a fresh VERSION and an EICAR canary returning FOUND succeed |
API tests with a fake clamd: an ERROR or timeout triggers SHUTDOWN, admission refuses until recovery, and a fake that answers OK to the canary is unavailable. A Helm test that the clamd entrypoint has the lifetime bound; a staging check that the restart count rises on schedule |
| 4d | Holding the inspect bootstrap secret allows at most the role's live-session cap, with one leased job per session at a time, and every session, session/renew, lease, bytes and result call is audit-logged. Only sessions renewed within the last 120 seconds count towards the cap. It does not allow sessions beyond the cap (refused, and alerted if refusals last more than 4 minutes), bytes for a job not leased to that session, a second concurrent lease, a session that has missed two renewals or passed its 15-minute expiry, the other role's calls, or any change to a scan result. Filling the cap blocks checks: this fails closed and raises an alert, and never makes anything previewable or approvable. During a rotation, both Secret names are accepted until the old one is removed |
API contract tests, with a fake clock, for each refusal, the cap and alert, renewal staleness, expiry and replacement by checkerId. A node-loss test: a pod's session stops renewing without ending; its replacement (a new pod name) gets 429 while the old session still counts, backs off (at most 60 seconds) and retries without exiting, and gets a session once the old one has missed two renewals, within the staleness window plus one backoff, measured from when the replacement pod starts (the test starts its clock there, not at the simulated node loss, because Kubernetes' eviction delay comes first). A cap-filled test: checks block, and the cap alert fires once refusals have lasted 4 minutes with no successful session for that role, with no proposal reaching checked. The node-loss test raises no cap alert. A rate-limited lease answers 204, and the worker keeps its session. An idle worker's lease backoff never exceeds 30 seconds, and a refused session call records no heartbeat. A log assertion for the audit entries. A test that the API accepts both configured names and then only the new one |
| 5 | With no checker heartbeat, a proposal is refused and nothing is claimed | API integration test |
| 6 | With the scanner unreachable, stale or future-dated, or with a truncated reply, the proposal is blocked, it is never offered to an inspector, and no preview exists | The scanner protocol tests from #3947, kept in the API, plus API tests for each unavailable case |
| 7 | An EICAR-style test file is failed as unsafe by the API's scan and never offered for inspection; a clean PDF with JavaScript, OpenAction script, an escaped-name script, embedded files or more than 200 pages is refused by the checker | An API test with a clamd test container; tests inside the checker image (the four #3947 renderer cases plus new ones) |
| 7a | A fabricated or forged checker result cannot turn an infected or unscanned file clean | API contract tests: for a proposal the API scanned as infected, and for one whose scan was unavailable, post a well-formed passed result (also with a forged scan: clean field) using a valid inspect token. Expect 409 or no effect, the proposal still failed or blocked, no Checked file and no preview |
| 8 | A result with a wrong SHA-256, stale lease, wrong attempt, unknown enum or contradictory facts never yields "checked"; a fetch token cannot call result or bytes, and an inspect token cannot call fetched |
API contract tests, including 403 for each cross-role call |
| 9 | A replayed identical result is a no-op; two replicas never lease the same job | API tests plus a physical Mongo concurrency test |
| 10 | A job that crashes or times out twice while the flag is on is failed; a job never leased within its 10-minute window is blocked; time spent with the flag off neither expires a job nor counts as an attempt, even when nothing called the API during the off period | API tests with a fake clock and an off period longer than the window, run three ways: with no thread reads and no checker polls (only the start-up mark); with checker polls answering 404; and with a rolling mix of on and off pods |
| 11 | Link fetching refuses private, metadata, CGNAT, IPv6 non-global and rebinding targets, and more than 3 redirects; it caps at 8 MiB and 30 s | The fetcher tests moved from #3947, plus a DNS-rebinding test |
| 12 | Approval publishes exactly the checked bytes (same SHA-256) without fetching again | #3947's existing physical and browser tests, rerun through the checker |
| 13 | Flag off: every proposal and checking endpoint, including session and session/renew, answers 404 and history is readable. Off, on, off, on with data made in each state works, and a job interrupted by "off" resumes. The worker never exits on a refusal, and none of these cases raises the replacement alert: (a) an off period shorter than 120 seconds, where the worker retries with its existing token and the same session carries on; (b) a rolling deploy with flag-on and flag-off pods, where a renewal that reaches a flag-off pod gets 404 and is retried on the same session; © an off period longer than 2 minutes, where the session goes stale, the next call gets 401, and the worker opens exactly one new session when the flag returns, then resumes leasing |
API round-trip test with a fake clock; checker tests that a 404 keeps the token and retries the same call, a 401 opens a new session, a 429 backs off and retries session, and none of them exits or opens an extra session; E2E with a checker in the local stack |
| 14 | The production render of the API chart is unchanged by L9 | Render production values before and after, and diff (expect none) |
| 15 | In staging, an uploaded PDF is checked end to end by the isolated checker and approved by an administrator | Staging run by Chris, recorded with proposal ids and SHA-256 |
| 16 | Production enablement waits for enforced NetworkPolicy and an operator security review | A checklist in the production enablement PR; no production config exists before then |
| 17 | With checkedPdfProposals on, a checked upload is counted and listed in slice 9's "Proposed PDFs to check" with source CheckedUpload and its check state; with it off, the count and the list are exactly as today. In both states slice 9's Use and Reject, and the legacy Approve and Reject, refuse it (409 study-pdf-proposal-checked), and it is decided only through the checked decision |
The repository and controller tests named in §6 ("Changes to slice 9's reads"): flag-off cases unchanged, flag-on cases new; the unchanged decision tests still pass; an explain test that both filters use the index |
| 18 | A proposed web address is never fetched by the server, with checkedPdfProposals on or off |
A test with a recording HTTP handler: no outbound request on propose, Use or Reject, in both flag states |
| 19 | With the flag on but the lease index missing, a proposal is refused before any claim and lease answers 204; once the index exists, admission opens without a restart |
API tests copying StudyPdfProposalsAdmission's tests: flag off, flag on with no index, a failed flag decision (off), and the index created after the first refusal |
| 20 | A Studies workspace proposal needs no issue: only a SubmitPdfCorrections holder on a live study of the route project may propose; a retry with the same request id finds the same proposal; claim, completion and decision write no issue post and no inbox notice |
API and physical Mongo tests for the Studies origin: 403 without the permission, 404 for another project's or a deleted study, the same id on retry, and no StudyIssue or InboxNotification write |
10. Slices¶
In dependency order, numbered P0 to P10 so they are not confused with the Studies workspace's slices. Sizes are rough: S under 300 changed lines, M 300 to 1,000, L over 1,000, tests and chart included. The MVP boundary is the end of P5.
| # | PR | Contents | Depends on | Size |
|---|---|---|---|---|
| P0 | This plan (syrf, docs only) | The design, the decisions and the questions | None | S |
| P1 | pdf-checker service (syrf) |
.NET worker and image (the only image with QPDF, Poppler and prlimit); inspect role only; CheckedPdfValidator and its tests moved from #3947; credential, non-dumpable and restart-after-abnormal-job hardening; a contract test against a fake API; chart with the inspect Deployment and clamd (API-only ingress), pod security and NetworkPolicies; .github/services.json, dependency map and a CI lane on juniper-ci. Deployed nowhere |
§3 contract agreed (this plan) | L |
| P2 | API checking pipeline (syrf) | checkedPdfProposals flag; quarantine file states; the API's own clamd scan (ClamdPdfVirusScanner and tests from #3947); CheckedPdfProposalRepository reworked without L8's issue model, with the origin (§3, §6); the private listener (chart value, default off) with session, session/renew, lease, bytes and result; the verdict, admission (flag and index, caps), heartbeat and activation records; the lease index by pre-deploy migration; guard tests that the API image has no native tools (AC 1, 2, 19). No screen yet |
§3 contract agreed | L (the larger end: the repository is reworked, not carried) |
| P3 | First screen: Propose with a file (syrf) | The Studies workspace's Propose takes a file (part of #4751), under the Studies origin (§3, AC 20); the upload goes to P2's quarantine; the card shows Checking, Blocked, Failed or Checked; slice 9's reads split as in §6 so checked uploads are listed as CheckedUpload with Preview and the checked Approve and Reject, and approval publishes the exact bytes through the accepted-PDF route (from #3947's AcceptedStudyPdfsController); slice 9's spec updated; local E2E with the checker in the stack. The file option, the card states and the list's new row are new screen states, built in Material 3 and shown to Chris for approval |
P2 merged; P1's image for E2E | L |
| P4 | cluster-gitops staging | Checker and clamd Applications for staging; the generated role secrets (ESO, Valkey pattern); the API checking listener on in staging values; no production entry | P1 and P2 merged (image and chart tags) | M |
| P5 | Staging switch-on (cluster-gitops) | checkedPdfProposals on in staging, after Chris approves; AC 15 run |
P3 and P4 | S |
| P6 | Administrators' Add and Replace with a file (syrf) | The rest of #4751 on the same pipeline | P3 | S to M |
| P7 | L9: issue-thread proposals (syrf) | #3947's issue-thread pieces rebuilt on main on top of L8 (§12) | L8 (#3945) merged; P2 | M |
| P8 | Network enforcement (camarades-infrastructure, cluster-gitops audit) | Audit the existing policies, then enable enforcement in Terraform (cluster-wide, separate approval) | None | M, high blast radius |
| P9 | Optional: fetch role for linked PDFs | §4, only if Chris asks for linked PDFs to be copied | P5 | M |
| P10 | Production | Operator review, production config and flag decision. Out of scope here | P8 and an operator review | — |
What can run in parallel. P1 and P2 can be built at the same time, because §3 fixes the contract between them; each tests against a fake of the other. P3 can start against P2's branch and merges after it. P4 can be drafted while P1 to P3 are in review and merges once their images exist. P8 can start at any time and gates only P10. After the MVP, P6 and P7 are independent of each other. The critical path to the MVP is P1 and P2 together, then P3 and P4, then P5.
Delivery status¶
| # | State | Where | Notes |
|---|---|---|---|
| P0 | Merged 10 November | #4683 | This plan |
| P1 | Merged 10 November | #4889 | src/services/pdf-checker: the inspect-role worker, its image (the only one with QPDF, Poppler and prlimit), the chart (inspect and clamd Deployments, NetworkPolicies) and its own CI lane pdf-checker-image.yml. Deployed nowhere. Meets AC 3, 4, 4a, 4b and the checker half of 7 and 13; see the service README. The contract examples both sides test against are in src/testing/pdf-checking-contract/v1, with the choices §3 left open listed in its README |
| P2 | Merged 11 November | #4888 | Flag checkedPdfProposals, migration 0039, the propose route (Studies origin, no screen), the private listener and the verdict; the decision is built in the repository, its route is P3's. Operator guide: Operate the checked PDF pipeline |
| P3 to P10 | Not started |
Review follow-ups for the merged parts are in #4894 (checker) and #4892 (API; both open). The lease timing gap in #4892 is to be fixed before P5.
11. Decisions and questions¶
11.1 Answered, with the source¶
The first draft had seven questions. Four are answered by owner decisions, standing rules or the code on main, and two more are design choices this plan can make. Each answer stands unless Chris says otherwise.
- Where PDF checking runs: the new checking service in our cluster (Option B). Chris decided on 4 November that
single uploaded PDFs go through one pipeline, this plan's checking service (D7 = A,
design decision log). Option A is ruled out by ADR-015
(the PDF agent is ARRNC-hosted, beside write access to every public PDF). Option C is ruled out for now by the
GitOps rule in
CLAUDE.md: its VPC, subnets and Cloud Run have no declarative owner, so it would need explicit approval first. - Must network enforcement come first? Moved to §11.2, question 1, because staging and production share one cluster (§0a).
- PDF checking gets its own switch,
checkedPdfProposals.CLAUDE.mdsays to flag a feature that is risky, spans services or wants a kill switch, and this one does all three. It also follows L7, where conversations got their own flag beside the study-attention family switch (feature-definitions.yaml,reconciliation-conversations), and the restart plan's "each behind its own flag" for L6 to L9. - Uploads first; links are not fetched. On main a proposed web address is already used as a link and never
fetched by the server (§0a), under Chris's D5 = A ("Propose takes a web address") and D6 = A (Studies slice 9 over
those proposals). So uploads are the only proposals that need checking. The fetch role (§4) stays an optional later
PR (P9), built only if Chris asks for linked PDFs to be copied into SyRF. This replaces the older notification
plan's "download once" for links (
docs/features/notification-inbox/technical-plan.md, opening section). - One virus scanner per environment, staging only for now. Each SyRF environment keeps its own data path
(
.claude/rules/pdf-agent.md,EnvironmentRoot), and production has no Application until a reviewed promotion. A shared scanner would let staging load affect production checks. It needs about 2 GiB per environment; staging's node headroom is a prerequisite to check (§8). - How the checker proves who it is: a generated secret per role. The External Secrets generator creates it, as
for Valkey (
docs/how-to/bff-session-store-valkey.md), so no person handles it and there is no dependency on the identity service. It is exchanged for short-lived sessions (§1, §3). Move to an identity-service client if more internal services need one. - The extra kernel sandbox (gVisor): not for staging; decided at the production review. It only matters once production is considered, which already waits for enforcement and an operator review (AC 16). It is on that review's checklist (§8).
11.2 Questions for Chris¶
Three questions. Each has a recommended answer, and the plan already uses it.
- Can staging start checking uploads before the cluster enforces network rules?
- What it is. Today any pod in the cluster can reach any other, in staging and production alike. The checker holds no passwords. But if a malicious PDF took it over, it could reach any service in the cluster that answers without a password, and send data out to the internet. Turning enforcement on is a cluster-wide change that rebuilds the nodes and starts enforcing three existing sets of rules, which need checking first (P8 in §10).
- Options. (a) Staging starts now, and enforcement is required before production. (b) Enforcement first, then staging.
- Recommendation: (a). The checker holds nothing worth stealing, staging is not mission-critical, and in staging the uploaders are you and the testers (as the first draft stated; not re-checked). Enforcement stays a hard condition for production.
- If you pick (b). P8 moves onto the critical path before P5, and its node rebuild needs its own approval.
- Where does an approved uploaded PDF live?
- What it is. Once an administrator approves a checked upload, SyRF has to store it somewhere it can serve it from. Your D5 said a single-file upload "scans and then copies, reusing the bulk stages", which would put it on the PDF server beside bulk-uploaded PDFs. D7 then sent the checking through this service.
- Options. (a) In SyRF's database, at most 8 MiB, served by the API at a fixed public address, as #3947 built it. This works in every environment now. (b) Copied to the PDF server like bulk uploads. That waits for the bulk PDF pipeline to be live in each environment, and production's is not. © Object storage, which needs a new way to serve it.
- Recommendation: (a) for now, with a later move to object storage or the PDF server kept as a follow-up (#3950 item 6). The checker only ever sees an API address, so a later move changes nothing else.
- If you pick (b) or ©. P3 gains a copy step and waits until that storage exists in staging.
- Which screen uses the checker first?
- What it is. The checker is shared. Its first user can be the Studies workspace's Propose with a file, or L9's issue-thread proposals, which wait for L8 (#3945) and its four open questions.
- Options. (a) Propose with a file in the Studies workspace first; L9's issue thread joins after L8. (b) L9's issue thread first, as first planned.
- What each costs. Either way, the administrator list you approved needs a change before it shows checked uploads: today it deliberately leaves them out, and its own plan gives that change to this work (§6). Option (a) also means taking L8's issue model out of #3947's proposal store now (§6), which makes P2 and P3 larger. Option (b) can keep that store closer to #3947, but the same change is still needed later, because your D7 sends the Studies workspace's uploads through this pipeline too.
- Recommendation: (a), unchanged but with a higher cost than the first revision said. It does not wait for L8, and the extra work is needed anyway. P3 is now an L-sized PR, not M to L.
- If you pick (b). P3 is replaced by P7, which waits for L8 to merge, so the MVP moves back by however long L8 takes. P2 gets a little smaller.
12. What carries over from #3947¶
3947 stays the source of most of the code, but not as a branch to merge: it is stacked on L8, far behind main, and¶
its central choice (the tools in the API) is the one this plan reverses. Each PR in §10 takes the pieces below from it.
| Piece on #3947's branch | Where it goes |
|---|---|
Services/CheckedPdf/CheckedPdfValidator.cs, CheckedPdfValidatorTests.cs and the four renderer tests in the generated runtime image |
P1, into pdf-checker, with #3950 item 12's prompt child termination |
Services/CheckedPdf/ClamdPdfVirusScanner.cs and ClamdPdfVirusScannerTests.cs |
P2, kept in the API (managed socket code) |
CheckedPdfProposal and CheckedPdfFile, ICheckedPdfProposalRepository |
P2, with the new additive fields and an optional IssueId (§6) |
CheckedPdfProposalRepository (claim, complete and decide transactions) and its physical Mongo tests |
P2, reworked: the transaction shapes carry over, but the StudyIssue reads and writes move behind the origin, because StudyIssue is L8's and not on main (§6). The tests are split the same way |
StudyPdfCorrection.BindCheckedProposal and the legacy Approve and Reject refusal |
P2, keeping the CheckedProposalId name main already reads |
AcceptedStudyPdfsController and its tests; the nginx upload location in src/services/web/default.conf.template |
P3 (if §11.2 question 2 is (a)) |
The upload, preview and decision actions of CheckedPdfProposalsController |
P3 for the Studies workspace; the issue-thread routes in P7 |
StudyIssuePostCapacity (history-capacity reservation) and the issue history and inbox capture |
P7, on top of L8 |
The checkedPdf clamd entries in env-mapping.yaml and _env-blocks.tpl |
P2 |
e2e/tests/checked-pdf-proposals.spec.ts |
P3 and P7, adapted |
PublicPdfDownloader and its address-range tests |
ValidUri and ReadBoundedAsync stay in the API (P2); the rest only in optional P9 |
checked-pdf.component.* and checked-pdf-api.service.ts |
Reference only: new screens are Material 3 on @camaradesuk/m3e-angular and need Chris's approval screen by screen |
docs/features/notification-inbox/checked-pdf-plan.md and the user-guide/notifications.md section |
Replaced by this plan; the user guide is rewritten with P3 and P7 |
Not carried. The apt-get install poppler-utils util-linux qpdf line in the API Dockerfile and the API's
runtime_packages generator change; the API registrations of the validator, downloader and DNS resolver; the
in-request checking and its SemaphoreSlim(2); and the generated files (api-client.generated.ts, swagger.json,
.generated-checksums.json, the flag consumer manifest), which are regenerated on main.