Fix sentry minified#4481
Conversation
WalkthroughSentry setup and sourcemap upload were moved out of the Dockerfile into two new scripts: Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
This PR has been deployed to https://linagora.github.io/tmail-flutter/4481. |
|
hoangdat
force-pushed
the
fix_sentry_minified
branch
from
cbc43eb to
c4daf96
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
scripts/configure-sentry.sh (2)
27-31: 💤 Low valueConsider sanitizing variables before interpolation in perl commands.
The environment variables (
SENTRY_PROJECT,SENTRY_ORG,SENTRY_RELEASE) are interpolated directly into perl regex patterns. If these contain special characters like|,$, or backticks, they could break the regex or cause unintended behavior.Since these values come from CI/CD build arguments, the risk is low in practice, but defensive quoting or validation would be safer.
♻️ Optional: Add basic validation
+# Validate inputs contain only safe characters (alphanumeric, dash, underscore, dot) +validate_input() { + case "$1" in + *[!a-zA-Z0-9._-]*) echo "Invalid characters in $2: $1" >&2; exit 1;; + esac +} + +validate_input "${SENTRY_PROJECT}" "SENTRY_PROJECT" +validate_input "${SENTRY_ORG}" "SENTRY_ORG" + perl -pi -e "s|^ project:.*| project: ${SENTRY_PROJECT}|" pubspec.yaml🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/configure-sentry.sh` around lines 27 - 31, In scripts/configure-sentry.sh the perl one-liners interpolate SENTRY_PROJECT, SENTRY_ORG, SENTRY_RELEASE, SENTRY_DIST directly which can break regexes if they contain metacharacters; update the script to sanitize/escape those env vars before using them (e.g., apply a quotemeta-style escape or validate the variables against an allowed character set) and then use the escaped value in the perl -pi -e substitutions for the lines that set project/org/release/dist/url_prefix so the regex patterns are safe even if the env values contain special characters.
20-21: 💤 Low valueVersion extraction is fragile if pubspec.yaml has unusual formatting.
The
grep "^version:" | tr -d ' 'approach assumes no leading spaces on theversion:line. This works for standard Flutter pubspec files but could silently produce wrong results if the format differs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/configure-sentry.sh` around lines 20 - 21, The version extraction using _pubspec_ver and the SENTRY_RELEASE assignment is fragile because grep "^version:" and tr -d ' ' assume no leading whitespace; update the extraction to robustly parse pubspec.yaml by matching optional leading spaces for the "version:" key and extracting the value correctly (preserving the '+' build metadata handling already used by SENTRY_RELEASE). Locate the _pubspec_ver assignment and the SENTRY_RELEASE="${SENTRY_RELEASE:-...}" line and replace the grep/tr approach with a command that accepts leading whitespace (e.g., use a more flexible grep/awk/sed expression that matches "^\s*version:" and reliably returns only the version token) so SENTRY_RELEASE is set from the normalized version string.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Dockerfile`:
- Around line 24-25: The flutter build web command in the Dockerfile removed the
build-time define for SENTRY_RELEASE; restore a dart-define for SENTRY_RELEASE
so the app receives the CI release string at build time (e.g. add
--dart-define=SENTRY_RELEASE=$SENTRY_RELEASE alongside the existing
--dart-define=SENTRY_DIST=$GITHUB_SHA) so sentry_config.dart and
SentryConfigLinagoraEcosystem continue to get the intended release value used
for symbol uploads.
---
Nitpick comments:
In `@scripts/configure-sentry.sh`:
- Around line 27-31: In scripts/configure-sentry.sh the perl one-liners
interpolate SENTRY_PROJECT, SENTRY_ORG, SENTRY_RELEASE, SENTRY_DIST directly
which can break regexes if they contain metacharacters; update the script to
sanitize/escape those env vars before using them (e.g., apply a quotemeta-style
escape or validate the variables against an allowed character set) and then use
the escaped value in the perl -pi -e substitutions for the lines that set
project/org/release/dist/url_prefix so the regex patterns are safe even if the
env values contain special characters.
- Around line 20-21: The version extraction using _pubspec_ver and the
SENTRY_RELEASE assignment is fragile because grep "^version:" and tr -d ' '
assume no leading whitespace; update the extraction to robustly parse
pubspec.yaml by matching optional leading spaces for the "version:" key and
extracting the value correctly (preserving the '+' build metadata handling
already used by SENTRY_RELEASE). Locate the _pubspec_ver assignment and the
SENTRY_RELEASE="${SENTRY_RELEASE:-...}" line and replace the grep/tr approach
with a command that accepts leading whitespace (e.g., use a more flexible
grep/awk/sed expression that matches "^\s*version:" and reliably returns only
the version token) so SENTRY_RELEASE is set from the normalized version string.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 15628e7d-1f88-4671-b5f2-30e00547cf93
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Dockerfilecore/lib/utils/sentry/sentry_initializer.dartpubspec.yamlscripts/configure-sentry.shscripts/run-sentry.sh
💤 Files with no reviewable changes (1)
- core/lib/utils/sentry/sentry_initializer.dart
hoangdat
force-pushed
the
fix_sentry_minified
branch
from
c4daf96 to
53bcb5c
Compare
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
5-5: ⚖️ Poor tradeoff
SENTRY_AUTH_TOKENpassed as a buildARGleaks into image layer metadata.Secrets passed as build arguments are recorded in the image history, making them retrievable by anyone who can access the image. The multi-stage build here only prevents the token from appearing in the final nginx
ENV, but a secret still exists in the builder stage layers and is extractable viadocker history --no-trunc. Secrets mounted with BuildKit are only accessible during the specificRUNinstruction and are never persisted in the final image.Replace the
ARG-based approach with a BuildKit secret mount:🔒 Proposed refactor using BuildKit secret mount
Remove
ARG SENTRY_AUTH_TOKEN(line 5) and theENVassignment (which already correctly omits it). Then change theRUNstep to:-ARG SENTRY_AUTH_TOKEN ARG SENTRY_ORG ...-RUN ./scripts/prebuild.sh && \ - SENTRY_AUTH_TOKEN=${SENTRY_AUTH_TOKEN:-} ./scripts/configure-sentry.sh && \ - flutter build web --release --source-maps \ - --dart-define=SENTRY_RELEASE=$SENTRY_RELEASE \ - --dart-define=SENTRY_DIST=$GITHUB_SHA && \ - SENTRY_AUTH_TOKEN=${SENTRY_AUTH_TOKEN:-} ./scripts/run-sentry.sh +RUN --mount=type=secret,id=sentry_auth_token,env=SENTRY_AUTH_TOKEN \ + ./scripts/prebuild.sh && \ + ./scripts/configure-sentry.sh && \ + flutter build web --release --source-maps \ + --dart-define=SENTRY_RELEASE=$SENTRY_RELEASE \ + --dart-define=SENTRY_DIST=$GITHUB_SHA && \ + ./scripts/run-sentry.shAnd update the build invocation in CI:
docker build \ --secret id=sentry_auth_token,env=SENTRY_AUTH_TOKEN \ ...Also applies to: 23-23, 27-27
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 5, Remove the ARG SENTRY_AUTH_TOKEN usage because it leaks into image layers; instead use a BuildKit secret mount for the RUN that needs the token. Concretely, delete the ARG SENTRY_AUTH_TOKEN declaration and any place that consumes it as a build ARG, update the Dockerfile RUN step that accesses the token to use a secret (mount type=secret) so the secret is only available during that RUN, and update the CI build invocation to pass the secret via docker build --secret id=sentry_auth_token,env=SENTRY_AUTH_TOKEN (or equivalent) rather than --build-arg; reference the Dockerfile symbol ARG SENTRY_AUTH_TOKEN and the RUN step that previously used it when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Dockerfile`:
- Line 5: Remove the ARG SENTRY_AUTH_TOKEN usage because it leaks into image
layers; instead use a BuildKit secret mount for the RUN that needs the token.
Concretely, delete the ARG SENTRY_AUTH_TOKEN declaration and any place that
consumes it as a build ARG, update the Dockerfile RUN step that accesses the
token to use a secret (mount type=secret) so the secret is only available during
that RUN, and update the CI build invocation to pass the secret via docker build
--secret id=sentry_auth_token,env=SENTRY_AUTH_TOKEN (or equivalent) rather than
--build-arg; reference the Dockerfile symbol ARG SENTRY_AUTH_TOKEN and the RUN
step that previously used it when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c0e8edcd-9925-49d0-930e-fdc420506558
⛔ Files ignored due to path filters (1)
pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Dockerfilecore/lib/utils/sentry/sentry_initializer.dartpubspec.yamlscripts/configure-sentry.shscripts/run-sentry.sh
🚧 Files skipped from review as they are similar to previous changes (3)
- scripts/run-sentry.sh
- pubspec.yaml
- core/lib/utils/sentry/sentry_initializer.dart
|
3 participants
Summary by CodeRabbit