The main concern here isn’t really whether the agent needs access to the whole codebase. Personally I feel an agent might need to have access to all or most of the codebase to make better decision, see things have been done before etc.
The real issue is that containers are being used as a security boundary while it’s well known they are not. Containers aren't a sufficient isolation mechanism for multi-tenant / untrusted workloads.
Using them to run your code review agent again puts your customers source code at risk of theft, unless you are using an actual secure sandbox mechanism to protect your customers data which from reading the article does not seem to be the case.
The seems to be looking to let the agent access the source code for review. But in that case, the agent should only see the codebase and nothing else. For a code review agent, all it really needs are:
- Access to files in the repositorie(s)
- Access to the patch/diff being reviewed
- Ability to perform text/semantic search across the codebase
That doesn’t require running the agent inside a container on a system with sensitive data. Exposing an API to the agent that specifically give it access to the above data, avoiding the risk altogether.
If it's really important that the agent is able to use a shell, why not use something like codespaces and run it in there?
I guess maybe even more things? The approach presented in the article doesn't seem like a good way of giving access to these by the way. All of these don't live on a dev machine. Things like Github codespaces are better suited for this job and are in fact already used to implement code reviews by LLMs.
My point is whitelisting is better than blacklisting.
When a front end need access to a bunch of things in a database. We usually provide exactly what's needed through an API, we don't let it run SQL queries on the database and attempt to filter / sandbox the SQL queries.
They don’t violate the spirit of the language. They are optional. They don’t change the behaviour at runtime.
Type annotations can seem pointless indeed if you are unwilling to learn how to use them properly. Using a giant union to type your (generic) function is indeed silly, you just have to make that function generic as explained in another comment or I guess remove the type hints
- There is one obvious way to provide type hints for your code, it’s to use the typing module provided by the language which also provides syntax support for it.
- You don’t have to use it because not all code has to be typed
- You can use formatted strings, but you don’t have to
- You can use comprehensions but you don’t have to
- You can use async io, but you don’t have to. But it’s the one obvious way to do it in python
The obvious way to annotate a generic function isn’t with a giant Union, it’s with duck typing using a Protocol + TypeVar. Once you known that, the obvious way is… pretty obvious.
The obvious way not be bothered with type hints because you don’t like them is not to use them!
Python is full of optional stuff, dataclasses, named tuples, meta programming, multiple ancestor inheritance.
You dont have to use these features, but there are only one way to use them
"There should only be one way to do it" has not really been a thing in Python for at least the last decade or longer. It was originally meant as a counterpoint to Perl's "there's more than one way to do it," to show that the Python developers put a priority on quality and depth of features rather than quantity.
But times change and these days, Python is a much larger language with a bigger community, and there is a lot more cross-pollination between languages as basic philosophical differences between the most popular languages steadily erode until they all do pretty much the same things, just with different syntax.
> "There should only be one way to do it" has not really been a thing in Python for at least the last decade or longer.
It never was a thing in Python, it is a misquote of the Zen of Python that apparently became popular as a reaction against the TMTOWTDI motto of the Perl community.
How so? There is one way to do it. If you want typing, you use type hints. You wouldn't say that, say, functions are unpythonic because you can either use functions or not use them, therefore there's two ways to do things, would you?
And Python failed at that decades ago. People push terribly complicated, unreadable code under the guise of Pythonic. I disagree with using Pythonic as reasoning for anything.
This is a popular misquote from the Zen of Python. The actual quote is “There should be one—and preferably only one—obvious way to do it.”
The misquote shifts the emphasis to uniqueness rather than having an obvious way to accomplish goals, and is probably a result of people disliking the “There is more than one way to do it” adage of Perl (and embraced by the Ruby community) looking to the Zen to find a banner for their opposing camp.
And Python failed at that decades ago. People push terribly complicated, unreadable code under the guise of Pythonic. I disagree with using Pythonic as reasoning for anything.
Actually in Python it can. Since the type hints are accessible at runtime, library authors can for example change which values in kwargs are allowed based on the type of the argument.
So on the language level it doesn’t directly change the behavior, but it is possible to use the types to affect the way code works, which is unintuitive. I think it was a bad decision to allow this, and Python should have opted for a TypeScript style approach.
You can make it change the behaviour at runtime is different than it changes the behaviour at runtime I think?
Lots of very useful tooling such as dataclasses and framework like FastAPI rely on this and you're opinion is that it's a bad thing why?
In typescript the absence of type annotations reflection at runtime make it harder to implement things that people obviously want, example, interop between typescript and zod schemas. Zod resorts instead to have to hook in ts compiler to do these things.
I'm honestly not convinced Typescript is better in that particular area.
What python has opted for is to add first class support for type annotations in the language (which Javascript might end up doing as well, there are proposals for this, but without the metadata at runtime).
Having this metadata at runtime makes it possible to implement things like validation at runtime rather than having to write your types in two systems with or without codegen (if Python would have to resort to codegen to do this, like its necessary in typescript, I would personally find this less pythonic).
I think on the contrary it allows for building intuitive abstractions where typescript makes them harder to build?
Yeah, but then you get into the issues with when and where generic types are bound and narrowed, which can then make it more complicated, at which point one might be better off stepping back, redesigning, or letting go of perfect type hint coverage, for dynamic constructs, that one couldn't even write in another type safe language.
However it's worth point that due to the concurrency model of AWS Lambda (1 client request / ws message = 1 lambda invocation / one process only ever handles one request at a time before it can handle the next one), you would end up spawning much more AWS Lambda instances than you would with Cloudflare workers or Wasmer Edge.
There are cost implications obviously, but AWS lambda works this way also to make concurrency and scaling "simpler" by providing an easier mental model. Though much more expensive in theory
Just because you haven't heard of it doesn't mean the risk isn't real.
It's probably better to make some kind of risk assessment and decide whether you're willing to accept this risk for your users / business. And what you can do to mitigate this risk. The truth is the risk is always there and gets smaller as you add several isolation mechanisms to make it insignificant.
I think you meant “container escape is not as difficult as VM escape.”
A malicious workload doesn’t need to be root inside the container, the attack surface is the shared linux kernel.
Not allowing root in a container might mitigate a container getting root access outside of a namespace. But if an escape succeeds the attacker could leverage yet another privilege escalation mechanism to go from non-root to root
I've been using postgres as a local database for one of my personal projects, a GUI app or to run python tests that depend on it without having to rely on installing it in my environment.
I created a Python package that downloads an embedded Postgres binary, sets up the database, and gives you a database URL: https://github.com/kketch/tinypg
When not using python, been using this script to create ephemeral postgres databases for tests but also persistent one in my dev containers: https://github.com/eradman/ephemeralpg
I've wrapped it with yet another shell script to make it usable just like this:
You can do all of this using Rust today. Very sane access to multi-threading => writing rust code that runs in parallel and is compiled without you worrying about it to run under several web workers
Yes there is some glue needed. The amount of boilerplate needed can be reduced with a bundler or depending on the library you use (just like it is the case when you use workers with JavaScript on the web, you can use a bundler with support for them or plugins to make it less of a pain to setup).
wasm bindgen rayon provide support to use rayon in that context. There are also projects that provide lower level apis similar to std::thread. Std::thread will probably never support the wasm target.
It’s not WASI that gives you access to threading. Multi threading is a WebAssembly spec feature. Each thread is a separate web assembly module, they can share memory/code using SharedArrayBuffer if needed or WebAssembly tables for code.
The host runtime for WASM whether it’s the web or something like wasmtime is responsible for instantiating those module in Web workers on the Web (or node…) or in new OS threads for wasmtime. I guess this is the thick amount of glue needed
WASI does not implement POSIX. And it isn't the goal of WASI to implement POSIX.
It does not support pthreads either. WASI is similar to POSIX because its an API that provides a "system interface" for WASM programs. But the API is much simpler and quite different and capability based.
There are things like WASIX (by Wasmer) built on of WASI that aims to provide POSIX support for multiple languages
WASM itself does not support pthreads. However things like Emscripten have very good support for pthreads.
WASM is one of the compilation targets of Emscripten (we had asm.js before that) and on top of that emscripten provides excellent shims for many POSIX APIs.
Regarding Rust which has supported WASM as a compilation target for a long time now, the idiomatic way of doing this is not shiming, but using higher level APIs: if you need parallel execution threads and need to compile to WASM you would use a crate (such as rayon) that is able to compile to WASM (and will use web workers under the hood just like Emscripten does). You would not use pthreads directly (std::thread)
React is the front end framework I've used the most when doing front end. It is and was sold as a simple and performant library to build interactive web apps, but things like its virtual dom was a very memory-hungry abstraction that immediately needed fixing.
Over the years we've had a cascade of APIs to convince us that React was easy: shouldComponentUpdate, hooks / useEffect's manual dependency array. It always felt like React spent most of its time solving problems it didn't need to solve in the first place if he didn't create itself those problems by having a not so great hard to use API. State derivation and computed properties and dependency graphs were already solved problems before React came and tried to explain to everyone that it knew better. The irony is that the ecosystem is now going back on signals / observer approach.
Now that I've finished complaining, I will probably keep using it in the future! (or Preact! which i feel always done a better job of reimplementing React or more, the fresh framework from the same author while I think still WIP is really promising too).
The real issue is that containers are being used as a security boundary while it’s well known they are not. Containers aren't a sufficient isolation mechanism for multi-tenant / untrusted workloads.
Using them to run your code review agent again puts your customers source code at risk of theft, unless you are using an actual secure sandbox mechanism to protect your customers data which from reading the article does not seem to be the case.
reply