fix(file-service): restore configured S3 uploads - #264
devin-ai-integration[bot] wants to merge 2 commits into
Conversation
Co-Authored-By: bot_apk <apk@cognition.ai>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-Authored-By: bot_apk <apk@cognition.ai>
| ENV FILE_SHARE_EVENT_ALWAYS_FAIL=true | ||
|
|
||
| # With uploads failing permanently (above), users would have no files to share. | ||
| # Seeding gives every user a few demo documents on their first file listing, | ||
| # so the share-notification failure is demoable alongside the upload failure. | ||
| # so the share flow is demoable on an account that has uploaded nothing yet. | ||
| # docker-compose.yml passes false, so local stacks behave like the golden app. | ||
| ENV FILE_SEED_DEMO_DOCS=true |
There was a problem hiding this comment.
📝 Info: Sibling chaos flags remain baked in
The image still sets FILE_SHARE_EVENT_ALWAYS_FAIL=true and FILE_SEED_DEMO_DOCS=true (services/file-service/Dockerfile:27-32). The PR scopes itself to uploads, so share-failure and demo-seeding behavior persist by design.
Was this helpful? React with 👍 or 👎 to provide feedback.
| fn dockerfile_enables_upload_failure(dockerfile: &str) -> bool { | ||
| let logical_dockerfile = dockerfile.replace("\\\r\n", " ").replace("\\\n", " "); | ||
| logical_dockerfile.lines().any(|line| { | ||
| let instruction = line.trim(); | ||
| let mut parts = instruction.splitn(2, char::is_whitespace); | ||
| if !parts | ||
| .next() | ||
| .is_some_and(|part| part.eq_ignore_ascii_case("ENV")) | ||
| { | ||
| return false; | ||
| } | ||
|
|
||
| let fields: Vec<_> = parts | ||
| .next() | ||
| .unwrap_or_default() | ||
| .split_whitespace() | ||
| .collect(); | ||
| if fields.first().is_some_and(|field| field.contains('=')) { | ||
| return fields.iter().any(|field| { | ||
| field.split_once('=').is_some_and(|(name, value)| { | ||
| name == "FILE_UPLOAD_ALWAYS_FAIL" | ||
| && parse_bool(value.trim_matches(['"', '\'']), false) | ||
| }) | ||
| }); | ||
| } | ||
|
|
||
| fields.first() == Some(&"FILE_UPLOAD_ALWAYS_FAIL") | ||
| && fields | ||
| .get(1) | ||
| .is_some_and(|value| parse_bool(value.trim_matches(['"', '\'']), false)) | ||
| }) | ||
| } |
There was a problem hiding this comment.
📝 Info: Regression parser is best-effort
dockerfile_enables_upload_failure models only a subset of Docker ENV syntax (backslash continuations, simple quote stripping, legacy and equals forms). It is adequate as a guard against re-adding the truthy default but does not replicate Docker's full escaping rules.
Was this helpful? React with 👍 or 👎 to provide feedback.
Runtime verificationVerified the browser upload path against the local stack with
A pre-existing pagination issue prevents new uploads from appearing in the seeded Files grid after the first 50 records; search and the detail route confirm this upload exists. This is outside the PR scope. |
Summary
500 storage_errorbecause S3 rejects writes to a nonexistent bucket; all users of that image are affected.FILE_UPLOAD_ALWAYS_FAIL=true, soupload_fileignoresS3_BUCKETand redirects every upload.FILE_UPLOAD_ALWAYS_FAILdefaults in DockerENVinstructions while preserving explicit runtime overrides.cargo test(27 passed),cargo fmt --check, andcargo clippy --all-targets -- -D warnings; rebuilt the image and confirmed it contains no upload-failure env default. With Compose's explicit override removed from the test container, uploaded1761710476959.jpegthrough the browser, retrieved it through search/detail/download, and verified the LocalStack object bytes. A forced-true negative control reproduced the failure. Screen recording.Independent incident run; compare with #263. This PR uses a smaller Dockerfile guard while covering case-insensitive instructions, quoted truthy values, legacy syntax, multi-variable instructions, continuations, and hashes inside values.
Link to Devin session: https://app.devin.ai/sessions/c26a89f42c4b456b8fdd48e61b1f73c9