Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI/CD - Added Snyk C/C++ Scanning Job #1608

Merged

Conversation

dealako
Copy link
Contributor

@dealako dealako commented Jan 4, 2024

  • added example C/C++ Code scanner using the Snyk GitHub Action.
    The --unmanaged flag indicates this is for a C/C++ codebase. In this
    example, it currently scans on a new pull request to the 'main'
    branch. The repository administrator should set both the SNYK_ORG and
    SNYK_TOKEN environment variables before merging this PR. The
    environment variables can be obtained from the LFX Security team.

Signed-off-by: David Deal [email protected]


Copy link

linux-foundation-easycla bot commented Jan 4, 2024

CLA Signed


The committers listed above are authorized under a signed CLA.

@jmertic
Copy link
Contributor

jmertic commented Jan 4, 2024

Hey all - for context we are working on how to enable LFX Security to scan for C/C++ dependencies. @dealako put this together which he tested locally; if we can get this working here then we can look to roll out to other ASWF projects that are primarily C/C++ based. Let me know if this all makes sense :-)

@lgritz
Copy link
Contributor

lgritz commented Jan 4, 2024

Can you point us to an example of the kinds of problems this tries to identify in the code base?

And what does the "Monitor for NPM Vulnerabilities" step do, considering that this project doesn't use NPM in any way?

@JeanChristopheMorinPerso
Copy link
Member

@lgritz The main thing this checks is vulnerabilities in dependencies and I think it might also check licenses used, see https://docs.snyk.io/scan-using-snyk/supported-languages-and-frameworks/c-c++.

@dealako
Copy link
Contributor Author

dealako commented Jan 4, 2024

And what does the "Monitor for NPM Vulnerabilities" step do, considering that this project doesn't use NPM in any way?

Sorry - this is not related to NPM (copy/paste error). I will fix it in the PR update.

Can you point us to an example of the kinds of problems this tries to identify in the code base?

More information here: https://docs.snyk.io/scan-using-snyk/supported-languages-and-frameworks/c-c++

- added example C/C++ Code scanner using the Snyk GitHub Action.
  The `--unmanaged` flag indicates this is for a C/C++ codebase. In this
  example, it currently scans on a new pull request to the 'main'
  branch. The repository administrator should set both the SNYK_ORG and
  SNYK_TOKEN environment variables before merging this PR. The
  environment variables can be obtained from the LFX Security team.
- added *.h, *.c, *.cpp filter to only run the scan when source files
  are changed

Signed-off-by: David Deal <[email protected]>
@dealako
Copy link
Contributor Author

dealako commented Jan 4, 2024

I'm waiting for the CLA manager approvals 😩 (to resolve the EasyCLA check).

@dealako
Copy link
Contributor Author

dealako commented Jan 4, 2024

Expected usage and results:

❯ snyk test --unmanaged  --print-dep-paths --org=${SNYK_ORG}

Testing /Users/ddeal/projects/go/src/github.com/dealako/openexr...

Dependencies:

  https://github.com|academysoftwarefoundation/[email protected]
  purl: pkg:generic/[email protected]?download_url=https%3A%2F%2Fgithub.com%2FAcademySoftwareFoundation%2Fopenexr%2Farchive%2Fv3.2.1-rc.tar.gz
  confidence: 0.972
  matching files:
    - src/bin/exr2aces/main.cpp
    - src/bin/exrcheck/main.cpp
    - src/bin/exrenvmap/EnvmapImage.cpp
    ... and 411 more files

Tested 1 dependency for known issues, found 0 issues.

❯ snyk monitor --unmanaged --org=${SNYK_ORG}

Monitoring /Users/ddeal/projects/go/src/github.com/dealako/openexr (openexr)...

Explore this snapshot at https://app.snyk.io/org/openexr/project/82c0ea7b-6e7b-4cb3-b950-3137dc2dc158/history/0a62371c-1be6-4809-860a-bd3e84868b85

Notifications about newly disclosed issues related to these dependencies will be emailed to you.

@dealako dealako requested a review from cary-ilm January 4, 2024 22:39
@dealako
Copy link
Contributor Author

dealako commented Jan 10, 2024

@cary-ilm - any other changes or feedback?

Copy link
Member

@cary-ilm cary-ilm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cary-ilm cary-ilm merged commit e571107 into AcademySoftwareFoundation:main Jan 10, 2024
29 checks passed
@cary-ilm
Copy link
Member

@dealako,I neglected to set up the SNYK_ORG and SNYK_TOKEN vars before merging. I've filed a ticket with LF IT support.

@jmertic
Copy link
Contributor

jmertic commented Jan 11, 2024

Hey @cary-ilm - we did this at the org level - so nothing for you to worry about.

@cary-ilm
Copy link
Member

Got it, thanks!

@cary-ilm
Copy link
Member

Looks like the PR check (on #1604) is failing: https://github.com/AcademySoftwareFoundation/openexr/actions/runs/7490645510/job/20395759365?pr=1604

Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`

@jmertic, @dealako, are we still missing something?

@cary-ilm
Copy link
Member

@dealako, @jmertic, the Snyk checks are still failing with this error:

Run snyk auth 
  snyk auth 
  shell: /usr/bin/bash -e {0}
Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=1[2](https://github.com/AcademySoftwareFoundation/openexr/actions/runs/7612900352/job/20739105771?pr=1616#step:5:2)[3](https://github.com/AcademySoftwareFoundation/openexr/actions/runs/7612900352/job/20739105771?pr=1616#step:5:3)[4](https://github.com/AcademySoftwareFoundation/openexr/actions/runs/7612900352/job/20739105771?pr=1616#step:5:5)[5](https://github.com/AcademySoftwareFoundation/openexr/actions/runs/7612900352/job/20739105771?pr=1616#step:5:6)678`
Error: Process completed with exit code 2.

You implied this has been configured at the org level, are we still missing something here?

@jmertic
Copy link
Contributor

jmertic commented Jan 22, 2024

Hmm - this is added as an organization secret -> https://github.com/organizations/AcademySoftwareFoundation/settings/secrets/actions

@dealako Any suggestions? Maybe I put the wrong one in.

@jmertic
Copy link
Contributor

jmertic commented Jan 29, 2024

Hi @cary-ilm - looks like the problem was that I can't spell 🤦 .

Can you try again and see if it works? Sorry about all that!

@cary-ilm
Copy link
Member

What exactly did you change? I just re-ran one of the failed jobs and it failed again, but it's possible whatever you changed was not synced in?

@jmertic
Copy link
Contributor

jmertic commented Jan 30, 2024

I had fixed the secret to have it re-spelled correctly. Can you point me to the job run?

@cary-ilm
Copy link
Member

@jmertic
Copy link
Contributor

jmertic commented Jan 31, 2024

I believe this issue is related to the fact that pull_reuqest triggers from branches not in the org don't have access to secrets for security reasons.

Using pull_request_target, I think, gets around that, but I also wonder if the better practice is to run the job on cron schedule vs on PRs. Not sure what you would think about that @dealako

@dealako
Copy link
Contributor Author

dealako commented Feb 5, 2024

@jmertic - ah yes. You are correct: GitHub article.

A cron schedule is a reasonable option (possibly complimentary) as rarely updated code repositories wouldn't benefit from frequent security updates and advisories. Having a periodic cron job would flag any new vulnerabilities even when the repository doesn't receive daily or weekly pull requests.

@dealako
Copy link
Contributor Author

dealako commented Feb 5, 2024

LMK if you'd like me to submit a PR to run the scan daily...or weekly.

@jmertic
Copy link
Contributor

jmertic commented Feb 6, 2024 via email

@cary-ilm
Copy link
Member

cary-ilm commented Feb 8, 2024

+1 for a PR to run the scan weekly. Thanks!

@cary-ilm
Copy link
Member

cary-ilm commented Feb 9, 2024

The PR scan is causing the CI check to fail on every PR now. I'm going to remove the workflow file snyk-scan-pr.yml altogether. @dealako, when you submit a PR for a weekly scan, presumably it will be a different workflow anyway.

cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 9, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
@cary-ilm cary-ilm mentioned this pull request Feb 9, 2024
kdt3rd pushed a commit that referenced this pull request Feb 9, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 11, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
cary-ilm added a commit that referenced this pull request Feb 13, 2024
* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

* Bazel support: Bump Imath to 3.1.10 (#1626)

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Remove snyk-scan-pr.yml (#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>

* fix issue with unpacking sample counts (#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <[email protected]>

* adjust checks for core to better match c++ checks (#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <[email protected]>

* Fix install of symlink (#1633)

PR #1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <[email protected]>

* adds a shortcut to avoid reconstructing every call (#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <[email protected]>

* check and control reduceMemory and reduceTime in stream mode (#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in #1634

Signed-off-by: Kimball Thurston <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
cary-ilm pushed a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
- added example C/C++ Code scanner using the Snyk GitHub Action.
  The `--unmanaged` flag indicates this is for a C/C++ codebase. In this
  example, it currently scans on a new pull request to the 'main'
  branch. The repository administrator should set both the SNYK_ORG and
  SNYK_TOKEN environment variables before merging this PR. The
  environment variables can be obtained from the LFX Security team.
- added *.h, *.c, *.cpp filter to only run the scan when source files
  are changed

Signed-off-by: David Deal <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Feb 13, 2024
…1629)

* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

* Bazel support: Bump Imath to 3.1.10 (AcademySoftwareFoundation#1626)

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Remove snyk-scan-pr.yml (AcademySoftwareFoundation#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>

* fix issue with unpacking sample counts (AcademySoftwareFoundation#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <[email protected]>

* adjust checks for core to better match c++ checks (AcademySoftwareFoundation#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <[email protected]>

* Fix install of symlink (AcademySoftwareFoundation#1633)

PR AcademySoftwareFoundation#1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <[email protected]>

* adds a shortcut to avoid reconstructing every call (AcademySoftwareFoundation#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <[email protected]>

* check and control reduceMemory and reduceTime in stream mode (AcademySoftwareFoundation#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in AcademySoftwareFoundation#1634

Signed-off-by: Kimball Thurston <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
cary-ilm pushed a commit that referenced this pull request Feb 16, 2024
- added example C/C++ Code scanner using the Snyk GitHub Action.
  The `--unmanaged` flag indicates this is for a C/C++ codebase. In this
  example, it currently scans on a new pull request to the 'main'
  branch. The repository administrator should set both the SNYK_ORG and
  SNYK_TOKEN environment variables before merging this PR. The
  environment variables can be obtained from the LFX Security team.
- added *.h, *.c, *.cpp filter to only run the scan when source files
  are changed

Signed-off-by: David Deal <[email protected]>
cary-ilm added a commit that referenced this pull request Feb 16, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
cary-ilm added a commit that referenced this pull request Feb 16, 2024
* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

* Bazel support: Bump Imath to 3.1.10 (#1626)

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* Add uninstall target (#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Remove snyk-scan-pr.yml (#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on #1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>

* fix issue with unpacking sample counts (#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <[email protected]>

* adjust checks for core to better match c++ checks (#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <[email protected]>

* Fix install of symlink (#1633)

PR #1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <[email protected]>

* adds a shortcut to avoid reconstructing every call (#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <[email protected]>

* check and control reduceMemory and reduceTime in stream mode (#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in #1634

Signed-off-by: Kimball Thurston <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Mar 3, 2024
This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>
cary-ilm added a commit to cary-ilm/openexr that referenced this pull request Mar 3, 2024
…1629)

* Build python wheels via scikit-build-core

This converts the setuptools configuration for building python wheels
to use scikit-build-core, which has better support for CMake. There is
no more setup.py; the configuration is entirely in `pyproject.toml`
and the compile/link is done exclusively via cmake.

The build/publish policy is:

* A PR that changes any of the python wheel source/configuration
  (src/wrappers/python/* or .github/workflows/python-wheels.yml)
  triggers a build as a check.

* PRs that change other library source do *not* trigger a build of the
  python wheels. Note that the primary CI workflow does build and test
  the bindings, although only for a single python version on a single
  arch for Linux/macOS/Windows. The wheel building validates multiple
  python versions and architectures, but involves signifant
  computation/time.  Currently, the python wheels are a thin wrapper
  about basic read/write functions that don't add significant
  additional functionality to the library. Any potential problem will
  almost certainly get caught by the primary CI.

* A tag of the form `v3.[0-9]+.[0-9]+-rc*` (e.g. `v3.2.4-rc`) triggers
  a full build of the wheels followed by a publish to
  `test.pypi.org`. This validates release candidates.

* Publishing a release triggers a full build of the wheels followed by
  a publish to `pypi.org`.

Signed-off-by: Cary Phillips <[email protected]>

* Add custom README.md for pypi.org

Signed-off-by: Cary Phillips <[email protected]>

* fix typo

Signed-off-by: Cary Phillips <[email protected]>

* reference src/wrappers/python/README.md in pyproject.toml

Signed-off-by: Cary Phillips <[email protected]>

* Add copyright notice

Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update pyproject.toml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Update src/wrappers/python/CMakeLists.txt

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Add cmake.targets and OPENEXR_INSTALL=OFF

Signed-off-by: Cary Phillips <[email protected]>

* INSTALL_TOOLS=OFF

Signed-off-by: Cary Phillips <[email protected]>

* propogate OPENEXR_INSTALL to Imath

Signed-off-by: Cary Phillips <[email protected]>

* test1

Signed-off-by: Cary Phillips <[email protected]>

* OPENEXR_INSTALL_PKG_CONFIG

Signed-off-by: Cary Phillips <[email protected]>

* Fix CVE 2023 5841 (AcademySoftwareFoundation#1627)

* enable deep file checks for core

Signed-off-by: Kimball Thurston <[email protected]>

* fix possible int overflow

Signed-off-by: Kimball Thurston <[email protected]>

* fix validation of deep sample counts

Addresses CVE-2023-5841, fixing sample count check to not only check
against 0 but previous sample as well.

Signed-off-by: Kimball Thurston <[email protected]>

* add clarifying comment

Signed-off-by: Kimball Thurston <[email protected]>

---------

Signed-off-by: Kimball Thurston <[email protected]>

* Bazel support: Bump Imath to 3.1.10 (AcademySoftwareFoundation#1626)

Signed-off-by: Vertexwahn <[email protected]>

* Document security expectations (AcademySoftwareFoundation#1623)

* Document security expectations

Signed-off-by: Cary Phillips <[email protected]>

* Menion Imath as a dependency

Signed-off-by: Cary Phillips <[email protected]>

* Update SECURITY.md

Co-authored-by: Nick Porcino <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* change 'Threat Model' to 'Potential Vulnerabilties'

Signed-off-by: Cary Phillips <[email protected]>

* Mention GitHub issue as fallback security contact

Signed-off-by: Cary Phillips <[email protected]>

* github security advisory

Signed-off-by: Cary Phillips <[email protected]>

* mention exrcheck

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>

* Add uninstall target (AcademySoftwareFoundation#1624)

* Add uninstall target

Satisfy the OpenSSF Best Practices Badge requirement for an
insta/uninstall process:
https://www.bestpractices.dev/en/criteria/1#1.installation_common

CMake does not support a standard "uninstall" target, but the
community recommends implementing an "uninstal" target that remove files named in the
`install_manifest.txt`:
https://gitlab.kitware.com/cmake/community/-/wikis/FAQ#can-i-do-make-uninstall-with-cmake

However, our existing process of installing the symlink to the "bare"
library, i.e. the symlink from libImath-3_2.so to libImath.so, fails
to add the symlink to the manifest, so "make uninstall" misses the
symlink. The existing mechanism use "install(CODE execute_process(cmake -E create_symlink))".

This changes that to use a simpler "file(CREATE_LINK)" and
"install(FILES)" to accomplish the same thing while also registering
the symlink the the manifest.

Also, this fixes an issue where `OpenEXRConfig.h` was passed to
`install()` twice, producing two entries in `install_manifest.txt`.

Signed-off-by: Cary Phillips <[email protected]>

* mention uninstall in install instructions

Signed-off-by: Cary Phillips <[email protected]>

* poke

Signed-off-by: Cary Phillips <[email protected]>

* COPY_ON_ERROR

Signed-off-by: Cary Phillips <[email protected]>

* clarify the uninstall instructions

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>

* Remove snyk-scan-pr.yml (AcademySoftwareFoundation#1631)

This workflow is causing errors on each PR:

  Snyk is missing auth token in order to run inside CI. You must include your API token as an environment value: `SNYK_TOKEN=12345678`
  Error: Process completed with exit code 2.

As discussed on AcademySoftwareFoundation#1608, the preferred workflow will run weekly, not on PR.

Signed-off-by: Cary Phillips <[email protected]>

* fix issue with unpacking sample counts (AcademySoftwareFoundation#1630)

When unpacking sample counts as "individual" counts (no longer
monotonic), it writes the total sample count to a value 1 past the
individual sample counts, but this is not in the packed data, so do not
expect to unpack that many values. The buffer just needs to be allocated
one value larger to avoid write past end of buffer which is taken care
of in the update_pack_unpack_ptrs function

Signed-off-by: Kimball Thurston <[email protected]>

* adjust checks for core to better match c++ checks (AcademySoftwareFoundation#1632)

The core checks were not setting the same image / tile size limits and
not disabling reads at quite the same level.

Note: the core check does not read the entire image into a contiguous
slice, so does not replicate the maximum deep sample checks in the same
way, this is a source of potential false-negative failures

This should address OSS-Fuzz 66491 and 66489 (different forms of the
same failure where a large sample size allocation was happening), and
are only constrained memory (2.5Gb) issues.

Signed-off-by: Kimball Thurston <[email protected]>

* Fix install of symlink (AcademySoftwareFoundation#1633)

PR AcademySoftwareFoundation#1624 caused the .so symlink without the `OPENEXR_LIB_SUFFIX`
(e.g. libOpenEXR.so which links to libOpenEXR-3_2.so) to get created
in the wrong directory. This caused certain invocations of cmake to
fail, even though the invocation in the CI succeeded. It's not at all
clear why.

This also changes the CI to invoke cmake in the way that previously
failed (e.g. from the top-level directory with `-B` and `-S`), as an additional check.

Signed-off-by: Cary Phillips <[email protected]>

* adds a shortcut to avoid reconstructing every call (AcademySoftwareFoundation#1634)

When there is a loop trying to get scan / tile info that is ignoring
return values, add a shortcut to avoid trying to reconstruct the chunk
table every time. This will still respect the strict header flag, either
returning an error immediately (strict), or (non-strict) enabling a
multi-part file with only partially corrupt parts to work.

Signed-off-by: Kimball Thurston <[email protected]>

* check and control reduceMemory and reduceTime in stream mode (AcademySoftwareFoundation#1635)

exrcheck by default uses file mode, but the fuzzer and exrcheck -s use
stream mode, need to respect the memory and time flags consistently on
that path as well.

Will address OSS-Fuzz 66612, although real fix underlying is in AcademySoftwareFoundation#1634

Signed-off-by: Kimball Thurston <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* Add sdist

Signed-off-by: Cary Phillips <[email protected]>

* Update .github/workflows/python-wheels-publish-test.yml

Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Cary Phillips <[email protected]>

* fix sdist; remove debugging

Signed-off-by: Cary Phillips <[email protected]>

---------

Signed-off-by: Cary Phillips <[email protected]>
Signed-off-by: Kimball Thurston <[email protected]>
Signed-off-by: Vertexwahn <[email protected]>
Co-authored-by: Jean-Christophe Morin <[email protected]>
Co-authored-by: Kimball Thurston <[email protected]>
Co-authored-by: Vertexwahn <[email protected]>
Co-authored-by: Nick Porcino <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants