diff --git a/.env.example b/.env.example index f26920b..b63edda 100644 --- a/.env.example +++ b/.env.example @@ -8,9 +8,26 @@ PARSE_TIMEOUT_SECONDS=60 LINK_TTL_SECONDS=604800 MAX_PENDING_JOBS=100 # Production: APP_ENV=production and PUBLIC_BASE_URL=https://your-domain.example -# Optional private S3-compatible storage, same settings for API and workers: -# S3_BUCKET=private-schematics -# S3_ENDPOINT_URL=https://s3.example.com +# Storage: auto uses S3 when S3_BUCKET is set; otherwise local. Use s3 to require S3. +STORAGE_BACKEND=auto +S3_BUCKET= +# AWS S3: leave endpoint blank. S3-compatible provider: its service endpoint, not a public file URL. +S3_ENDPOINT_URL= +S3_REGION=us-east-1 +# Optional namespace inside the bucket, e.g. altium/production (empty preserves old S3 keys). +S3_PREFIX= +# auto, path (commonly MinIO/S3-compatible), or virtual +S3_ADDRESSING_STYLE=auto +S3_CONNECT_TIMEOUT=5 +S3_READ_TIMEOUT=60 +S3_MAX_ATTEMPTS=3 +# Credentials are server-only. Omit for IAM/workload credentials. # AWS_ACCESS_KEY_ID= # AWS_SECRET_ACCESS_KEY= -# AWS_DEFAULT_REGION=us-east-1 +# AWS_SESSION_TOKEN= +# Optional custom CA bundle path, available in both API/worker containers: +# AWS_CA_BUNDLE=/path/to/provider-ca.pem +# Optional encryption: empty (provider default), AES256 or aws:kms. +S3_SERVER_SIDE_ENCRYPTION= +S3_KMS_KEY_ID= +# API and workers must use exactly the same bucket/prefix/endpoint/region. diff --git a/README.md b/README.md index 978c604..cadbd37 100644 --- a/README.md +++ b/README.md @@ -98,12 +98,18 @@ The workspace displays original/modified SVGs side by side with synchronized zoo ## Storage and configuration -Local files are private, written atomically with generated keys. Client filenames never determine storage paths. For private S3-compatible storage set the same `S3_BUCKET`, `S3_ENDPOINT_URL`, AWS credentials and region on API and workers; no bucket ACL or public URL is created. Prefer workload credentials/IAM and configure bucket encryption/lifecycle at the provider. On a multi-host deployment use S3; a Docker named volume is shared only on one host. Storage/database orphan reconciliation after a machine failure is an operational maintenance task; normal completion, retries and expiry cleanup are implemented. +Local files are private, written atomically with generated keys. Client filenames never determine storage paths. For private S3-compatible storage set `STORAGE_BACKEND=s3` and the same `S3_BUCKET`, `S3_PREFIX`, `S3_ENDPOINT_URL`, credentials and region on API and workers; no bucket ACL or public URL is created. Prefer workload credentials/IAM and configure bucket encryption/lifecycle at the provider. On a multi-host deployment use S3; a Docker named volume is shared only on one host. Storage/database orphan reconciliation after a machine failure is an operational maintenance task; normal completion, retries and expiry cleanup are implemented. + +Full settings, credentials, optional encryption, migration instructions and the local-versus-temporary-file boundary are documented in [S3 storage](docs/S3.md). Existing local files are not moved automatically; a verified, non-deleting migration utility is provided. | Environment | Default | | --- | --- | | `DATABASE_URL` | SQLite under `STORAGE_DIR`; Compose uses PostgreSQL | -| `STORAGE_DIR` | `./data` (`/data` in Compose) | +| `STORAGE_DIR` | `./data` (`/data` in Compose); local objects / SQLite metadata only | +| `STORAGE_BACKEND` | `auto`: S3 when bucket is set, local otherwise; `s3` requires S3 | +| `S3_BUCKET` / `S3_PREFIX` | Existing private bucket / optional object namespace | +| `S3_ENDPOINT_URL` / `S3_REGION` | Provider endpoint / region (AWS default endpoint when blank) | +| `S3_ADDRESSING_STYLE` | `auto`, `path` or `virtual` | | `API_KEY` | Empty in local development; required in production | | `PUBLIC_BASE_URL` | `http://127.0.0.1:8000` | | `MAX_UPLOAD_BYTES` | 33,554,432 (32 MiB) | diff --git a/backend/app/config.py b/backend/app/config.py index b68f61a..52cdf6b 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -3,6 +3,7 @@ import os from dataclasses import dataclass from functools import lru_cache from pathlib import Path +from urllib.parse import urlsplit @dataclass(frozen=True) class Settings: @@ -18,10 +19,30 @@ class Settings: s3_bucket: str s3_endpoint: str | None s3_region: str + storage_backend: str + s3_prefix: str + s3_addressing_style: str + s3_connect_timeout: int + s3_read_timeout: int + s3_max_attempts: int + s3_encryption: str + s3_kms_key_id: str @lru_cache def settings(): root = Path(os.getenv('STORAGE_DIR', './data')).resolve() + bucket = os.getenv('S3_BUCKET', '').strip() + backend = os.getenv('STORAGE_BACKEND', 'auto').strip().lower() + if backend == 'auto': backend = 's3' if bucket else 'local' + if backend not in ('local', 's3'): + raise RuntimeError('STORAGE_BACKEND must be auto, local or s3') + if backend == 's3' and not bucket: + raise RuntimeError('S3_BUCKET is required when STORAGE_BACKEND=s3') + prefix = os.getenv('S3_PREFIX', '').strip('/') + if (len(prefix.encode('utf-8')) > 512 or '\\' in prefix or + any(ord(c) < 32 or ord(c) == 127 for c in prefix) or + (prefix and any(part in ('', '.', '..') for part in prefix.split('/')))): + raise RuntimeError('S3_PREFIX must be a relative object prefix without empty, dot or traversal segments (max 512 UTF-8 bytes)') config = Settings( environment=os.getenv('APP_ENV', 'development'), database_url=os.getenv('DATABASE_URL', f'sqlite:///{root}/jobs.db'), @@ -32,9 +53,17 @@ def settings(): parse_timeout=int(os.getenv('PARSE_TIMEOUT_SECONDS', '60')), link_ttl=int(os.getenv('LINK_TTL_SECONDS', '604800')), max_pending=int(os.getenv('MAX_PENDING_JOBS', '100')), - s3_bucket=os.getenv('S3_BUCKET', ''), + s3_bucket=bucket, s3_endpoint=os.getenv('S3_ENDPOINT_URL') or None, - s3_region=os.getenv('AWS_DEFAULT_REGION', 'us-east-1'), + s3_region=os.getenv('S3_REGION') or os.getenv('AWS_DEFAULT_REGION') or 'us-east-1', + storage_backend=backend, + s3_prefix=prefix, + s3_addressing_style=os.getenv('S3_ADDRESSING_STYLE', 'auto').strip().lower(), + s3_connect_timeout=int(os.getenv('S3_CONNECT_TIMEOUT', '5')), + s3_read_timeout=int(os.getenv('S3_READ_TIMEOUT', '60')), + s3_max_attempts=int(os.getenv('S3_MAX_ATTEMPTS', '3')), + s3_encryption=os.getenv('S3_SERVER_SIDE_ENCRYPTION', '').strip(), + s3_kms_key_id=os.getenv('S3_KMS_KEY_ID', '').strip(), ) if config.environment == 'production' and len(config.api_key) < 32: raise RuntimeError('Production requires API_KEY with at least 32 characters') @@ -42,5 +71,18 @@ def settings(): raise RuntimeError('Production requires an HTTPS PUBLIC_BASE_URL') if min(config.max_upload_bytes, config.parse_timeout, config.link_ttl, config.max_pending) <= 0: raise RuntimeError('Upload, parse, expiry and queue limits must be positive') - root.mkdir(parents=True, exist_ok=True, mode=0o700) + if config.s3_addressing_style not in ('auto', 'path', 'virtual'): + raise RuntimeError('S3_ADDRESSING_STYLE must be auto, path or virtual') + if min(config.s3_connect_timeout, config.s3_read_timeout, config.s3_max_attempts) <= 0: + raise RuntimeError('S3 timeouts and max attempts must be positive') + if config.s3_encryption not in ('', 'AES256', 'aws:kms'): + raise RuntimeError('S3_SERVER_SIDE_ENCRYPTION must be empty, AES256 or aws:kms') + if config.s3_kms_key_id and config.s3_encryption != 'aws:kms': + raise RuntimeError('S3_KMS_KEY_ID requires S3_SERVER_SIDE_ENCRYPTION=aws:kms') + if config.s3_endpoint: + url = urlsplit(config.s3_endpoint) + if url.scheme not in ('http', 'https') or not url.hostname or url.username or url.password or url.query or url.fragment: + raise RuntimeError('S3_ENDPOINT_URL must be an HTTP(S) service endpoint without credentials, query or fragment') + if backend == 'local' or config.database_url.startswith('sqlite:'): + root.mkdir(parents=True, exist_ok=True, mode=0o700) return config diff --git a/backend/app/storage.py b/backend/app/storage.py index 7d88d4c..2283312 100644 --- a/backend/app/storage.py +++ b/backend/app/storage.py @@ -11,11 +11,22 @@ KEY = re.compile(r'^[a-f0-9]{48}/(?:input\.(?:SchDoc|PcbDoc)|(?:old|new)\.(?:Sch class Storage: def __init__(self): self.config = settings() - if self.config.s3_bucket: + if self.config.storage_backend == 's3': import boto3 - self.s3 = boto3.client('s3', endpoint_url=self.config.s3_endpoint, region_name=self.config.s3_region) + from botocore.config import Config + self.s3 = boto3.client('s3', endpoint_url=self.config.s3_endpoint, region_name=self.config.s3_region, + config=Config(signature_version='s3v4', + s3={'addressing_style': self.config.s3_addressing_style}, + connect_timeout=self.config.s3_connect_timeout, + read_timeout=self.config.s3_read_timeout, + retries={'mode': 'standard', 'total_max_attempts': self.config.s3_max_attempts})) else: self.s3 = None + def object_key(self, key): + """Prefix exists only at the storage boundary, never in API IDs or DB keys.""" + if not KEY.fullmatch(key): raise ValueError('Invalid storage key') + return f'{self.config.s3_prefix}/{key}' if self.config.s3_prefix else key + def path(self, key): if not KEY.fullmatch(key): raise ValueError('Invalid storage key') p = self.config.storage_dir / 'objects' / key @@ -24,10 +35,14 @@ class Storage: return p def put(self, key, data, content_type='application/octet-stream'): - p = self.path(key) # validate on both backends - if self.s3: - self.s3.put_object(Bucket=self.config.s3_bucket, Key=key, Body=data, ContentType=content_type) + remote_key = self.object_key(key) + if self.s3 is not None: + options = {} + if self.config.s3_encryption: options['ServerSideEncryption'] = self.config.s3_encryption + if self.config.s3_kms_key_id: options['SSEKMSKeyId'] = self.config.s3_kms_key_id + self.s3.put_object(Bucket=self.config.s3_bucket, Key=remote_key, Body=data, ContentType=content_type, **options) else: + p = self.path(key) p.parent.mkdir(parents=True, exist_ok=True, mode=0o700) tmp = p.with_name(p.name + '.' + secrets.token_hex(8) + '.tmp') try: @@ -38,16 +53,16 @@ class Storage: finally: tmp.unlink(missing_ok=True) def get(self, key): - p = self.path(key) - if self.s3: - with self.s3.get_object(Bucket=self.config.s3_bucket, Key=key)['Body'] as body: + remote_key = self.object_key(key) + if self.s3 is not None: + with self.s3.get_object(Bucket=self.config.s3_bucket, Key=remote_key)['Body'] as body: return body.read() - return p.read_bytes() + return self.path(key).read_bytes() def delete(self, key): - p = self.path(key) - if self.s3: self.s3.delete_object(Bucket=self.config.s3_bucket, Key=key) - else: p.unlink(missing_ok=True) + remote_key = self.object_key(key) + if self.s3 is not None: self.s3.delete_object(Bucket=self.config.s3_bucket, Key=remote_key) + else: self.path(key).unlink(missing_ok=True) @lru_cache def storage(): return Storage() diff --git a/backend/tests/test_platform.py b/backend/tests/test_platform.py index ed7e6e8..cd0c2ed 100644 --- a/backend/tests/test_platform.py +++ b/backend/tests/test_platform.py @@ -15,6 +15,7 @@ KEY = 'test-api-key-' + 'x'*40 @pytest.fixture def client(tmp_path, monkeypatch): + monkeypatch.setenv('STORAGE_BACKEND', 'local') monkeypatch.setenv('STORAGE_DIR', str(tmp_path)) monkeypatch.setenv('DATABASE_URL', f'sqlite:///{tmp_path}/test.db') monkeypatch.setenv('API_KEY', KEY) diff --git a/backend/tests/test_storage_s3.py b/backend/tests/test_storage_s3.py index b369e15..a970f23 100644 --- a/backend/tests/test_storage_s3.py +++ b/backend/tests/test_storage_s3.py @@ -1,25 +1,174 @@ -"""Protocol-level S3 contract check with botocore's validation stub (no live S3).""" +"""S3 protocol/configuration and real API/worker flows with a fake S3 transport. + +No request is sent to a live provider and no real credential is required. +""" import io -import pytest +import importlib.util +from pathlib import Path +import time import boto3 +from botocore.exceptions import ClientError, EndpointConnectionError from botocore.response import StreamingBody from botocore.stub import Stubber -from app.storage import Storage +import pytest +from sqlalchemy import update +from app.config import settings +from app.storage import Storage, storage +from test_platform import client, KEY as API_KEY + +NAMES=['input.SchDoc','input.PcbDoc','old.SchDoc','new.SchDoc','old.PcbDoc','new.PcbDoc', + 'comparison.json','pcb-comparison.json','attempt-1/data.json','attempt-1/drawing.svg', + 'attempt-1/old.svg','attempt-2/new.svg'] -@pytest.mark.parametrize('name', ['input.SchDoc', 'input.PcbDoc', 'old.PcbDoc', 'new.PcbDoc', 'pcb-comparison.json', 'attempt-1/old.svg']) -def test_s3_private_round_trip_contract(tmp_path,monkeypatch,name): - from app.config import settings - from app.storage import storage - monkeypatch.setenv('STORAGE_DIR',str(tmp_path)); monkeypatch.setenv('S3_BUCKET','test-private') - settings.cache_clear(); storage.cache_clear() +@pytest.fixture +def s3_config(tmp_path,monkeypatch): + # Explicit fake credentials/client prevent the SDK's metadata discovery network calls. + for name in ['STORAGE_BACKEND','S3_BUCKET','S3_PREFIX','S3_REGION','S3_ENDPOINT_URL','S3_ADDRESSING_STYLE', + 'S3_CONNECT_TIMEOUT','S3_READ_TIMEOUT','S3_MAX_ATTEMPTS','S3_SERVER_SIDE_ENCRYPTION','S3_KMS_KEY_ID']: + monkeypatch.delenv(name,raising=False) + monkeypatch.setenv('STORAGE_DIR',str(tmp_path)) + monkeypatch.setenv('DATABASE_URL',f'sqlite:///{tmp_path}/storage.db') + monkeypatch.setenv('APP_ENV','development') + monkeypatch.setenv('STORAGE_BACKEND','s3');monkeypatch.setenv('S3_BUCKET','test-private') client=boto3.client('s3',region_name='us-east-1',aws_access_key_id='test',aws_secret_access_key='test') - monkeypatch.setattr(boto3,'client',lambda *a,**kw:client) - key='a'*48+'/'+name; payload=b'actual bytes' + captures=[] + def make_client(*args,**kwargs):captures.append((args,kwargs));return client + monkeypatch.setattr(boto3,'client',make_client) + settings.cache_clear();storage.cache_clear() + yield client,captures,tmp_path + settings.cache_clear();storage.cache_clear() + + +@pytest.mark.parametrize('name',NAMES) +@pytest.mark.parametrize('prefix',['','altium/production','/altium/production/']) +def test_s3_private_round_trip_contract(s3_config,monkeypatch,name,prefix): + client,_,root=s3_config + monkeypatch.setenv('S3_PREFIX',prefix);settings.cache_clear() + key='a'*48+'/'+name;remote=prefix.strip('/')+'/'+key if prefix else key;payload=b'actual bytes' with Stubber(client) as stub: - stub.add_response('put_object',{}, {'Bucket':'test-private','Key':key,'Body':payload,'ContentType':'application/octet-stream'}) - stub.add_response('get_object',{'Body':StreamingBody(io.BytesIO(payload),len(payload))},{'Bucket':'test-private','Key':key}) - stub.add_response('delete_object',{}, {'Bucket':'test-private','Key':key}) + stub.add_response('put_object',{},dict(Bucket='test-private',Key=remote,Body=payload,ContentType='application/octet-stream')) + stub.add_response('get_object',{'Body':StreamingBody(io.BytesIO(payload),len(payload))},dict(Bucket='test-private',Key=remote)) + stub.add_response('delete_object',{},dict(Bucket='test-private',Key=remote)) service=Storage();service.put(key,payload);assert service.get(key)==payload;service.delete(key) stub.assert_no_pending_responses() - settings.cache_clear(); storage.cache_clear() + assert not (root/'objects').exists() + + +@pytest.mark.parametrize('mode,kms,options',[ + ('','',{}),('AES256','',{'ServerSideEncryption':'AES256'}), + ('aws:kms','alias/altium',{'ServerSideEncryption':'aws:kms','SSEKMSKeyId':'alias/altium'}), +]) +def test_encryption_config_and_sdk_transport(s3_config,monkeypatch,mode,kms,options): + client,captures,_=s3_config + for key,value in {'S3_ENDPOINT_URL':'https://objects.example.test','S3_REGION':'test-region', + 'S3_ADDRESSING_STYLE':'path','S3_CONNECT_TIMEOUT':'7','S3_READ_TIMEOUT':'31','S3_MAX_ATTEMPTS':'4', + 'S3_SERVER_SIDE_ENCRYPTION':mode,'S3_KMS_KEY_ID':kms}.items():monkeypatch.setenv(key,value) + settings.cache_clear();key='a'*48+'/input.PcbDoc' + with Stubber(client) as stub: + stub.add_response('put_object',{},dict(Bucket='test-private',Key=key,Body=b'pcb',ContentType='application/octet-stream',**options)) + Storage().put(key,b'pcb');stub.assert_no_pending_responses() + args,kwargs=captures[0];assert args==('s3',) + assert kwargs['endpoint_url']=='https://objects.example.test' and kwargs['region_name']=='test-region' + config=kwargs['config'];assert config.signature_version=='s3v4' and config.s3['addressing_style']=='path' + assert config.connect_timeout==7 and config.read_timeout==31 and config.retries['total_max_attempts']==4 + assert not {'aws_access_key_id','aws_secret_access_key'} & kwargs.keys() + + +@pytest.mark.parametrize('key,value',[ + ('STORAGE_BACKEND','unknown'),('S3_BUCKET',''),('S3_PREFIX','../escape'),('S3_PREFIX','nested/../escape'), + ('S3_PREFIX','a//b'),('S3_PREFIX','bad\\prefix'),('S3_PREFIX','bad\nname'),('S3_PREFIX','x'*513), + ('S3_ADDRESSING_STYLE','invalid'),('S3_CONNECT_TIMEOUT','0'),('S3_READ_TIMEOUT','-1'),('S3_MAX_ATTEMPTS','0'), + ('S3_SERVER_SIDE_ENCRYPTION','public'),('S3_KMS_KEY_ID','alias/missing-encryption-mode'), + ('S3_ENDPOINT_URL','file:///tmp/data'),('S3_ENDPOINT_URL','https://user:secret@example.test'), + ('S3_ENDPOINT_URL','https://example.test/?token=secret'), +]) +def test_invalid_s3_configuration_fails_closed(s3_config,monkeypatch,key,value): + monkeypatch.setenv(key,value);settings.cache_clear() + with pytest.raises(RuntimeError):settings() + + +def test_auto_backend_compatibility_and_no_local_directory_with_postgres(s3_config,monkeypatch): + _,_,root=s3_config + monkeypatch.delenv('STORAGE_BACKEND');settings.cache_clear() + assert settings().storage_backend=='s3' + monkeypatch.setenv('S3_BUCKET','');settings.cache_clear() + assert settings().storage_backend=='local' + monkeypatch.setenv('STORAGE_BACKEND','s3');monkeypatch.setenv('S3_BUCKET','test-private') + monkeypatch.setenv('DATABASE_URL','postgresql+psycopg://unused:unused@localhost/unused') + monkeypatch.setenv('STORAGE_DIR',str(root/'unused-local-dir'));settings.cache_clear() + assert settings().storage_backend=='s3' and not (root/'unused-local-dir').exists() + + +def test_s3_errors_never_fall_back_to_disk(s3_config,monkeypatch): + client,_,root=s3_config + key='a'*48+'/input.SchDoc' + with Stubber(client) as stub: + stub.add_client_error('put_object',service_error_code='AccessDenied',http_status_code=403) + stub.add_client_error('get_object',service_error_code='AccessDenied',http_status_code=403) + stub.add_client_error('delete_object',service_error_code='AccessDenied',http_status_code=403) + service=Storage() + for operation in (lambda:service.put(key,b'x'),lambda:service.get(key),lambda:service.delete(key)): + with pytest.raises(ClientError):operation() + assert not (root/'objects').exists() + for key in ['../input.SchDoc','a'*48+'/input.SchDoc/other','foreign/'+'a'*48+'/input.PcbDoc']: + with pytest.raises(ValueError):service.object_key(key) + + +class MemoryS3: + """Transport fake only: all file validation/parsing/rendering/worker logic is real.""" + def __init__(self):self.objects={};self.content_types={} + def put_object(self,Bucket,Key,Body,ContentType,**kwargs): + self.objects[(Bucket,Key)]=bytes(Body);self.content_types[(Bucket,Key)]=ContentType + assert 'ACL' not in kwargs + def get_object(self,Bucket,Key): + if (Bucket,Key) not in self.objects: + raise ClientError({'Error':{'Code':'NoSuchKey'},'ResponseMetadata':{'HTTPStatusCode':404}},'GetObject') + data=self.objects[(Bucket,Key)];return {'Body':StreamingBody(io.BytesIO(data),len(data))} + def delete_object(self,Bucket,Key):self.objects.pop((Bucket,Key),None) + + +@pytest.mark.parametrize('kind',['schematic_render','schematic_comparison','pcb_render','pcb_comparison']) +def test_api_worker_read_and_expiry_cleanup_use_only_prefixed_s3(client,monkeypatch,ascii_schematic,kind): + from app.db import Job,claim,sessions + from app.worker import process + from app.cleanup import cleanup + from pcb_fixture import pcb + fake=MemoryS3();monkeypatch.setattr(boto3,'client',lambda *a,**kw:fake) + monkeypatch.setenv('STORAGE_BACKEND','s3');monkeypatch.setenv('S3_BUCKET','test-private');monkeypatch.setenv('S3_PREFIX','altium/test') + settings.cache_clear();storage.cache_clear() + pcb_kind=kind.startswith('pcb');comparison=kind.endswith('comparison') + suffix='PcbDoc' if pcb_kind else 'SchDoc';raw=pcb() if pcb_kind else ascii_schematic + route=('/api/v1/pcbs/comparisons' if comparison else '/api/v1/pcbs/render') if pcb_kind else ('/api/v1/comparisons' if comparison else '/api/v1/schematics/render') + files={f'{side}_file':(side+'.'+suffix,raw) for side in ('old','new')} if comparison else {'file':('input.'+suffix,raw)} + response=client.post(route,headers={'X-API-Key':API_KEY},files=files) + assert response.status_code==202,response.text + created=response.json();identifier=created['id'];headers={'X-Viewer-Token':created['viewer_token']} + fake.objects[('test-private','other-prefix/keep')]=b'other service data' + process(claim());status=client.get(created['status_url'],headers=headers).json() + assert status['status']=='completed',status + base=created['status_url'].removesuffix('/status') + assert client.get(base+('/svg/old' if comparison else '/svg'),headers=headers).status_code==200 + assert client.get(created['status_url']).status_code==404 + if comparison:assert status['total_changed']==0 + else:assert client.get(base+'/data',headers=headers).status_code==200 + assert all(key.startswith('altium/test/'+identifier+'/') for bucket,key in fake.objects if key!='other-prefix/keep') + assert not (settings().storage_dir/'objects').exists() + with sessions().begin() as db:db.execute(update(Job).where(Job.id==identifier).values(expires_at=time.time()-1)) + assert cleanup()==1 + assert fake.objects=={('test-private','other-prefix/keep'):b'other service data'} + + +def test_migration_copies_verifies_skips_identical_and_refuses_overwrite(s3_config,monkeypatch,tmp_path): + fake=MemoryS3();monkeypatch.setattr(boto3,'client',lambda *a,**kw:fake) + monkeypatch.setenv('S3_PREFIX','altium/test');settings.cache_clear() + script=Path(__file__).resolve().parents[2]/'scripts/migrate-to-s3.py' + spec=importlib.util.spec_from_file_location('migration',script);module=importlib.util.module_from_spec(spec);spec.loader.exec_module(module) + key='a'*48+'/input.PcbDoc';source=tmp_path/'source';file=source/'objects'/key + file.parent.mkdir(parents=True);file.write_bytes(b'original') + assert module.migrate(source)=={'copied_and_verified':1,'already_identical':0} + assert file.read_bytes()==b'original' + assert module.migrate(source)=={'copied_and_verified':0,'already_identical':1} + fake.objects[('test-private','altium/test/'+key)]=b'different' + with pytest.raises(ValueError,match='refusing to overwrite'):module.migrate(source) + assert file.read_bytes()==b'original' diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index b66b2f1..f1c95da 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -23,7 +23,7 @@ The queue is in PostgreSQL, using `FOR UPDATE SKIP LOCKED` and a conditional upd API and worker can scale independently. Worker parsing is serial per worker container; increase replicas for concurrency. Parser subprocesses inherit no API key, database URL or storage credentials, receive files in an ephemeral directory, and run under CPU/address-space/file-size/FD limits. The decoder never fetches document URLs or loads dynamic code. This resource boundary is defense in depth; hostile-file deployments may additionally isolate parsers with a hardened runtime/network namespace. -Local object paths are application-generated and validated. Files are mode 0600 in directories created as 0700. Compose uses the same persistent volume and UID for API/worker. For multiple hosts replace local storage with an S3-compatible private bucket. Database metadata and objects must be backed up consistently. Credentials are environment settings, not committed files or source-derived values. +Local object paths are application-generated and validated. Files are mode 0600 in directories created as 0700. Compose uses the same persistent volume and UID for API/worker. For multiple hosts select `STORAGE_BACKEND=s3` with an S3-compatible private bucket and a shared `S3_PREFIX`; no document objects are written locally in this mode. See [configuration and migration](S3.md). Database metadata and objects must be backed up consistently. Credentials are environment settings, not committed files or source-derived values. The normalized document keeps original property maps, owner relationships and active parts separately from drawables. This is necessary for diagnostics and future parser improvements: the SVG is an output, not the only stored interpretation. Source paint order is preserved. Geometry is not laid out by a graph library. diff --git a/docs/S3.md b/docs/S3.md new file mode 100644 index 0000000..2f84641 --- /dev/null +++ b/docs/S3.md @@ -0,0 +1,124 @@ +# Private S3 object storage + +Schematic and PCB uploads, comparison manifests, normalized JSON and SVG artifacts use the same storage adapter. Configure it once for both API and worker. No API endpoint, viewer URL, token or database schema changes are required. + +## Configure `.env` + +```dotenv +STORAGE_BACKEND=s3 +S3_BUCKET=your-private-bucket +S3_PREFIX=altium/production +S3_ENDPOINT_URL=https://your-s3-service.example +S3_REGION=us-east-1 +S3_ADDRESSING_STYLE=path +AWS_ACCESS_KEY_ID=your-access-key +AWS_SECRET_ACCESS_KEY=your-secret-key +S3_CONNECT_TIMEOUT=5 +S3_READ_TIMEOUT=60 +S3_MAX_ATTEMPTS=3 +S3_SERVER_SIDE_ENCRYPTION= +S3_KMS_KEY_ID= +``` + +Replace the example values with your provider's actual bucket, service endpoint, credentials and region. The bucket must already exist. The application never creates a bucket, changes its ACL/policy or makes objects public. Keep the bucket private. API-controlled access and expiry continue to apply; the browser does not connect directly to S3 or receive storage credentials. + +For AWS S3, leave `S3_ENDPOINT_URL` empty and choose the bucket's region; `auto` addressing is normally suitable. For MinIO and S3-compatible providers use the provider's endpoint and addressing style (often `path`). Do not put the bucket name, access credentials, query parameters or a public asset URL into `S3_ENDPOINT_URL`. + +| Variable | Default / behavior | +| --- | --- | +| `STORAGE_BACKEND` | `auto`: S3 if `S3_BUCKET` is nonempty, local otherwise. `s3` requires a bucket and never falls back to disk. `local` explicitly selects local files. | +| `S3_BUCKET` | Existing private bucket name, not a URL | +| `S3_PREFIX` | Empty for backward compatibility; optional namespace, e.g. `altium/production` | +| `S3_ENDPOINT_URL` | Empty uses the AWS endpoint selected by the SDK | +| `S3_REGION` | Falls back to `AWS_DEFAULT_REGION`, then `us-east-1` | +| `S3_ADDRESSING_STYLE` | `auto`, `path` or `virtual`; default `auto` | +| `S3_CONNECT_TIMEOUT` | Positive integer seconds, default 5 | +| `S3_READ_TIMEOUT` | Positive integer seconds, default 60 | +| `S3_MAX_ATTEMPTS` | Positive integer, default 3 total attempts including initial request; standard SDK retry mode | +| `AWS_ACCESS_KEY_ID` | Standard SDK credential chain; no credential is embedded in source | +| `AWS_SECRET_ACCESS_KEY` | Standard SDK credential chain | +| `AWS_SESSION_TOKEN` | Optional for temporary credentials | +| `AWS_CA_BUNDLE` | Optional trusted custom CA PEM path accessible to API and worker; certificate verification remains enabled | +| `S3_SERVER_SIDE_ENCRYPTION` | Empty uses provider defaults; optional `AES256` or `aws:kms` | +| `S3_KMS_KEY_ID` | Optional KMS key/alias, valid only with `aws:kms`; requires provider support and key permissions | + +IAM/workload credentials can be used instead of static keys. Do not set empty static credentials when using an IAM role. Keep `.env` private and ignored by Git; do not put credentials in frontend code or `VITE_*` variables. The parser subprocess inherits neither AWS credentials nor storage configuration. + +Some compatible providers do not implement newer optional AWS checksum behavior. Where the provider documents this requirement, the SDK's standard `AWS_REQUEST_CHECKSUM_CALCULATION=when_required` / `AWS_RESPONSE_CHECKSUM_VALIDATION=when_required` environment settings can be used without changing application code. + +Prefix rules: leading/trailing `/` is stripped; internal empty segments, `.`, `..`, backslashes, controls and prefixes exceeding 512 UTF-8 bytes are rejected. The prefix is a server-controlled namespace, never supplied by a request. Generated suffixes retain the existing strict allowlist. For example: + +```text +s3://your-private-bucket/altium/production/{id}/input.SchDoc +s3://your-private-bucket/altium/production/{id}/input.PcbDoc +s3://your-private-bucket/altium/production/{id}/comparison.json +s3://your-private-bucket/altium/production/{id}/pcb-comparison.json +s3://your-private-bucket/altium/production/{id}/old.PcbDoc +s3://your-private-bucket/altium/production/{id}/new.PcbDoc +s3://your-private-bucket/altium/production/{id}/attempt-1/data.json +s3://your-private-bucket/altium/production/{id}/attempt-1/drawing.svg +s3://your-private-bucket/altium/production/{id}/attempt-1/old.svg +s3://your-private-bucket/altium/production/{id}/attempt-1/new.svg +``` + +The database continues storing logical keys such as `{id}/input.PcbDoc`. Prefix application happens exactly once, at the S3 put/get/delete boundary. Expiry cleanup uses the same mapping and never lists or bulk-deletes a bucket or another prefix. + +## What stays outside S3 + +S3 replaces persistent **document object storage**, not the database. PostgreSQL still stores jobs, authorization hashes, expiry, queue leases and status. SQLite development databases remain local. Keep the same database when switching backends so existing IDs/tokens remain valid. + +Multipart spooling and resource-limited parsing still use temporary local files, normally under `/tmp`, and remove them after processing. They are not durable artifact storage. In S3 mode no `STORAGE_DIR/objects` files are created/read as fallback. With PostgreSQL the adapter does not require a local storage directory. Existing Compose data volumes are retained for safe migration/rollback; they no longer receive document objects once S3 is selected. Do not delete a previous volume until its objects have been migrated and backups verified. + +The bucket/prefix must be identical on all API and worker replicas. Bucket errors propagate through the existing storage error responses or durable failed-job status. An S3 outage does not switch new uploads to local storage. Object retention/lifecycle policies must not remove data earlier than the application link lifetime; database rows and S3 objects should be backed up together. + +Grant the service object permissions scoped to the chosen namespace: `s3:PutObject`, `s3:GetObject` and `s3:DeleteObject`. The application does not require public ACLs or bucket-wide listing for normal operations. Encryption/KMS may require additional provider permissions. Administrative backup and lifecycle tooling can have separate permissions. + +## Existing files and migration + +Changing backend/bucket/prefix does **not** move existing objects automatically. Existing S3 deployments keep working with `STORAGE_BACKEND=auto` and empty `S3_PREFIX`. If introducing a prefix for previously unprefixed S3 objects, first copy the existing keys to that namespace using your provider's tools while retaining the original suffixes. + +For local-to-S3 migration, `scripts/migrate-to-s3.py`: + +- Reads only allowlisted application objects from the original `STORAGE_DIR/objects` tree. +- Copies into the configured private S3 namespace and reads each copy back to verify its SHA-256. +- Skips an already identical destination; refuses to overwrite different bytes. +- Rejects symlinks/unrecognized paths and stops on provider errors. +- Never deletes local files or changes database rows. Completed copies remain if a later object fails, allowing a safe rerun. + +Stop admission of uploads and all workers/cleanup while copying so neither side changes under migration. Keep the original database and object volume. + +For local execution, load the edited `.env` and point to the **old** storage directory: + +```bash +set -a +source .env +set +a +PYTHONPATH=backend .venv/bin/python scripts/migrate-to-s3.py --source-dir ./data +``` + +For the existing Compose deployment (`/data` is the old named volume): + +```bash +docker compose stop api worker +docker compose build api worker +docker compose run --rm --no-deps \ + -v "$PWD/scripts:/tools:ro" \ + api python /tools/migrate-to-s3.py --source-dir /data +docker compose up -d api worker +``` + +Only proceed to the final startup after the copy command succeeds. Do not run production workers against new S3 settings before old objects have been copied; otherwise already queued jobs cannot find their sources. + +For a new installation with no old local objects, migration is unnecessary: + +```bash +docker compose up -d --build api worker +``` + +Compose already passes `.env` to both services. `./scripts/dev.sh` also exports `.env` for both local processes. Direct `uvicorn`/worker commands need the variables exported explicitly. Editing `.env` requires recreating Compose containers (a plain `docker compose restart` does not load new environment values). + +After switching, upload a SchDoc and PcbDoc, check their status and private SVGs, and open an existing pre-migration link. Confirm objects appear only under the configured bucket/prefix. No real provider was configured during implementation; protocol tests use botocore stubs and the end-to-end API/worker tests use a fake S3 transport with the real parsers/renderers. + +## SDK references + +[Configuration](https://docs.aws.amazon.com/boto3/latest/guide/configuration.html), [credential discovery](https://docs.aws.amazon.com/boto3/latest/guide/credentials.html), and [put_object/encryption parameters](https://docs.aws.amazon.com/boto3/latest/reference/services/s3/client/put_object.html). The application retains its existing pinned boto3 runtime dependency. diff --git a/scripts/migrate-to-s3.py b/scripts/migrate-to-s3.py new file mode 100644 index 0000000..a0348a5 --- /dev/null +++ b/scripts/migrate-to-s3.py @@ -0,0 +1,48 @@ +#!/usr/bin/env python3 +"""Copy and verify existing private local objects into the configured S3 namespace. + +No local objects or database rows are deleted or changed. Stop uploads/workers +before copying and keep them stopped until both switch to the same S3 settings. +Run with PYTHONPATH=backend and the service .env exported in the environment. +""" +import argparse +import hashlib +from pathlib import Path +from botocore.exceptions import ClientError +from app.config import settings +from app.storage import KEY, storage + + +def migrate(source_dir): + config=settings() + if config.storage_backend!='s3':raise ValueError('Configure STORAGE_BACKEND=s3 and S3_BUCKET before migrating') + root=Path(source_dir).resolve()/'objects' + if not root.is_dir() or root.is_symlink():raise ValueError('Source directory must contain the original objects directory (not a symlink)') + service=storage();copied=existing=0 + for path in sorted(root.rglob('*')): + if path.is_symlink():raise ValueError('Symlinks are not permitted in the source object tree') + if not path.is_file():continue + key=path.relative_to(root).as_posix() + if not KEY.fullmatch(key):raise ValueError('Unrecognized source object; migration stopped without deleting any files') + data=path.read_bytes();digest=hashlib.sha256(data).digest() + try:remote=service.get(key) + except ClientError as exc: + if exc.response.get('Error',{}).get('Code') not in ('NoSuchKey','404','NotFound'):raise + else: + if hashlib.sha256(remote).digest()!=digest: + raise ValueError(f'Destination object differs; refusing to overwrite {key}') + existing+=1;continue + content_type='application/json' if key.endswith('.json') else 'image/svg+xml' if key.endswith('.svg') else 'application/octet-stream' + service.put(key,data,content_type) + if hashlib.sha256(service.get(key)).digest()!=digest:raise ValueError('Destination verification failed') + copied+=1 + return {'copied_and_verified':copied,'already_identical':existing} + + +def main(): + parser=argparse.ArgumentParser(description=__doc__) + parser.add_argument('--source-dir',type=Path,default=None,help='Old STORAGE_DIR, e.g. ./data or /data; must contain objects/') + args=parser.parse_args() + print(migrate(args.source_dir or settings().storage_dir)) + +if __name__=='__main__':main()