Skip to content

Commit 46d07d3

Browse files
fix(lib-inject): set correct permissions on package files [backport 1.20] (#7252)
Backport 4b8f2f0 from #7239 to 1.20. Package files should not be world-writable. Users noticed injected ddtrace package files being flagged by security scanners. In particular the `bytecode` package in Python 3.7 includes world-writable sources files. ## Testing The existing tests should cover that the feature still works in the K8s library injection. The existing assertion for the permissions on the files is updated to assert that the permission is correctly applied to all package files. For the deb/rpm I manually tested the commands that the build runs and validated that the files are not world-writable. ## Risk There may be unknown situations where Python packages require these permissions. In the worst-case with these situations the application may crash due to an unexpected exception at runtime. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) are followed. If no release note is required, add label `changelog/no-changelog`. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). - [x] Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. - [x] Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) - [x] If this PR touches code that signs or publishes builds or packages, or handles credentials of any kind, I've requested a review from `@DataDog/security-design-and-guidance`. - [x] This PR doesn't touch any of that. Co-authored-by: Kyle Verhoog <[email protected]>
1 parent 1392007 commit 46d07d3

File tree

4 files changed

+13
-3
lines changed

4 files changed

+13
-3
lines changed

.github/workflows/lib-injection.yml

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,10 @@ jobs:
8888
cd lib-injection
8989
# Ensure /datadog-lib/ddtrace_pkgs is a valid directory that is not empty
9090
docker-compose run lib_inject find /datadog-init/ddtrace_pkgs -maxdepth 0 -empty | wc -l && if [ $? -ne 0 ]; then exit 1; fi
91-
# Ensure non-datadog users have read and write permissions to files stored in /datadog-lib/ddtrace_pkgs
92-
docker-compose run lib_inject find /datadog-init/ddtrace_pkgs ! -perm -a=rw | wc -l && if [ $? -ne 0 ]; then exit 1; fi
91+
# Ensure files are not world writeable
92+
docker-compose run lib_inject find /datadog-init/ddtrace_pkgs ! -perm /o+w | wc -l && if [ $? -ne 0 ]; then exit 1; fi
93+
# Ensure non-datadog users have read permissions to files stored in /datadog-lib/ddtrace_pkgs
94+
docker-compose run lib_inject find /datadog-init/ddtrace_pkgs ! -perm -a=r | wc -l && if [ $? -ne 0 ]; then exit 1; fi
9395
- name: Test the app
9496
run: |
9597
curl http://localhost:18080

.gitlab/build-deb-rpm.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,9 @@ echo `pwd`
3333

3434
cp ../lib-injection/sitecustomize.py dd-trace.dir/lib/
3535

36+
chmod -R o-w dd-trace.dir/lib
37+
chmod -R g-w dd-trace.dir/lib
38+
3639
cp auto_inject-python.version dd-trace.dir/lib/version
3740

3841
fpm_wrapper "datadog-apm-library-python" "$PYTHON_PACKAGE_VERSION" \

lib-injection/Dockerfile

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@ ARG UID=10000
2929
RUN addgroup -g 10000 -S datadog && \
3030
adduser -u ${UID} -S datadog -G datadog
3131
RUN chown -R datadog:datadog /datadog-init/ddtrace_pkgs
32-
RUN chmod -R a+rw /datadog-init/ddtrace_pkgs
32+
RUN chmod -R o-w /datadog-init/ddtrace_pkgs
33+
RUN chmod -R g-w /datadog-init/ddtrace_pkgs
3334
USER ${UID}
3435
WORKDIR /datadog-init
3536
ADD sitecustomize.py /datadog-init/sitecustomize.py
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
fixes:
3+
- |
4+
lib-injection: Update package files to not be world-writable.

0 commit comments

Comments
 (0)